Skip to content

fix: recognize script as module for Typescript type checking #604

New issue

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

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 1, 2024

Conversation

baseballyama
Copy link
Member

@baseballyama baseballyama commented Dec 1, 2024

close: #557

This PR adjusts the script to always be recognized as a module. This ensures that the error “Cannot redeclare block-scoped variable ‘xxx’” does not occur.

Copy link

changeset-bot bot commented Dec 1, 2024

🦋 Changeset detected

Latest commit: 6055811

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte-eslint-parser Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -95,7 +95,6 @@
"eslint-plugin-regexp": "^2.7.0",
"eslint-plugin-svelte": "^2.46.1",
"eslint-plugin-yml": "^1.15.0",
"estree-walker": "^3.0.3",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't use this dependency.

@@ -380,7 +398,7 @@ function analyzeRuneVariables(
// See https://github.com/sveltejs/svelte/blob/41b5cd6f5daae3970a9927e062f42b6b62440d16/packages/svelte/types/index.d.ts#L2615
case "$props": {
// Use type parameters to avoid `@typescript-eslint/no-unsafe-assignment` errors.
appendDeclareFunctionVirtualScripts(globalName, ["<T>(): T"]);
appendDeclareFunctionVirtualScripts(globalName, ["(): any"]);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

@baseballyama baseballyama Dec 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But 1 test is failed, so I reverted it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is intentionally typed differently than Svelte core.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 12102873057

Details

  • 23 of 23 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.06%) to 89.923%

Files with Coverage Reduction New Missed Lines %
src/parser/typescript/analyze/index.ts 1 90.07%
Totals Coverage Status
Change from base Build 12100948993: 0.06%
Covered Lines: 3013
Relevant Lines: 3232

💛 - Coveralls

Comment on lines +83 to +87
// When performing type checking on TypeScript code that is not a module, the error `Cannot redeclare block-scoped variable 'xxx'`. occurs. To fix this, add an `export`.
// see: https://github.com/sveltejs/svelte-eslint-parser/issues/557
if (!hasExportDeclaration(result.ast)) {
appendDummyExport(ctx);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is main part of this PR.

Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ota-meshi ota-meshi merged commit 5ed0609 into main Dec 1, 2024
10 checks passed
@ota-meshi ota-meshi deleted the fix/557 branch December 1, 2024 08:09
@AlbertMarashi
Copy link

nice!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conflict with lib.dom.d.ts when using runes
4 participants