-
-
Notifications
You must be signed in to change notification settings - Fork 22
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: resolve to module scope for top level statements #291
Conversation
🦋 Changeset detectedLatest commit: 4e3686f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
tests/src/scope/scope.ts
Outdated
@@ -0,0 +1,61 @@ | |||
import assert from "assert"; | |||
import { parseForESLint } from "../../../src"; | |||
import { getScopeFromNode } from "../../../src/scope"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR.
Can you replace the test with ESLint's context.getScope
instead of getScopeFromNode
? getScopeFromNode
is not used in the actual rule implementation.
So I dug deeper and the problem is that the |
@@ -0,0 +1,69 @@ | |||
import assert from "assert"; | |||
import * as svelte from "../../../src"; | |||
import { FlatESLint } from 'eslint/use-at-your-own-risk'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you change it to use the Linter
class instead of FlatESLint
? I believe that using the Linter
class is sufficient for this test.
Hmm... Fixing this bug seems a bit difficult. I would like to close this PR and work on this fix. |
Fixes #280.
scopeManager.acquire(node)
returns null in these cases.The fix is to only return the global scope for the root
Program
node (consistent with other eslint parsers) andSvelteScriptElement
(as I would expect it as it is for me also a root node)Some notes: