Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: allow JS snapshots within TS plugin#2565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
dummdidumm wants to merge1 commit intomaster
base:master
Choose a base branch
Loading
fromts-plugin-js-snapshots

Conversation

dummdidumm
Copy link
Member

So far we've always assumed that a Svelte file is a TS file for simplicity, due to our default language (which was removed a long time ago) and also because IIRC there were issues with TS having its snapshot scriptKind being switched. That seems to be no longer the case, and so we can get better at properly analyzing whether or not this is a TS file, to allow JSDoc to take action when it's a JS file.

#2555

@jasonlyu123 do you remember why that scriptKind switch broke previously, but apparently doesn't anymore? I vaguely remember us having the same problem within the language server, but you PRd a change at some point stating that it's no longer a problem.

Also, is this a breaking change, because if someone has noallowJS in their config they would get errors?

So far we've always assumed that a Svelte file is a TS file for simplicity, due to our default language (which was removed a long time ago) and also because IIRC there were issues with TS having its snapshot scriptKind being switched. That seems to be no longer the case, and so we can get better at properly analyzing whether or not this is a TS file, to allow JSDoc to take action when it's a JS file.#2555
@dummdidummdummdidumm marked this pull request as draftNovember 5, 2024 17:15
@jasonlyu123
Copy link
Member

jasonlyu123 commentedNov 6, 2024•
edited
Loading

Yes, there is aDebug.assert for ScriptKind changes. TypeScript added a fix for it some time ago. TheDebug.assert is still there, so it might still happen in some edge cases, but it shouldn't happen in most cases.#2538 is an example. In ts-plugin, you'll need file save to trigger updates, so for#2538 to happen in ts-plugin, it is edge case + edge case. Also, The more reliable reproduction for#2538 should only happen in module resolution "node16"+ because of the impliedNodeFormat patch but we didn't patch it in ts-plugin.

As forallowJs, I am leaning toward this as a correctness fix. It also should only affect files loaded through import and not in project files because we can't know if the files are lang="ts" without reading it.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@dummdidumm@jasonlyu123
[8]ページ先頭
©2009-2025 Movatter.jp