Skip to content
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

feat: Add svelte/valid-context-access rule #480

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Prev Previous commit
Next Next commit
support setTimeout and others
baseballyama committed Jun 3, 2023
commit 736e5a11deeba579f017c4eaf2dec941da62eadb
2 changes: 1 addition & 1 deletion src/rules/infinite-reactive-loop.ts
Original file line number Diff line number Diff line change
@@ -357,7 +357,7 @@ export default createRule("infinite-reactive-loop", {
const tickCallExpressions = Array.from(
extractSvelteLifeCycleReferences(context, ["tick"]),
)
const taskReferences = extractTaskReferences(context)
const taskReferences = Array.from(extractTaskReferences(context))
const reactiveVariableReferences = getReactiveVariableReferences(context)

return {
32 changes: 22 additions & 10 deletions src/rules/reference-helpers/microtask.ts
Original file line number Diff line number Diff line change
@@ -2,24 +2,36 @@ import type { TSESTree } from "@typescript-eslint/types"
import { ReferenceTracker } from "@eslint-community/eslint-utils"
import type { RuleContext } from "../../types"

type FunctionName = "setTimeout" | "setInterval" | "queueMicrotask"

/**
* Get usage of `setTimeout`, `setInterval`, `queueMicrotask`
*/
export function extractTaskReferences(
export function* extractTaskReferences(
context: RuleContext,
): { node: TSESTree.CallExpression; name: string }[] {
functionNames: FunctionName[] = [
"setTimeout",
"setInterval",
"queueMicrotask",
],
): Generator<{ node: TSESTree.CallExpression; name: string }, void> {
const referenceTracker = new ReferenceTracker(
context.getSourceCode().scopeManager.globalScope!,
)
const a = referenceTracker.iterateGlobalReferences({
setTimeout: { [ReferenceTracker.CALL]: true },
setInterval: { [ReferenceTracker.CALL]: true },
queueMicrotask: { [ReferenceTracker.CALL]: true },
})
return Array.from(a).map(({ node, path }) => {
return {
for (const { node, path } of referenceTracker.iterateGlobalReferences({
setTimeout: {
[ReferenceTracker.CALL]: functionNames.includes("setTimeout"),
},
setInterval: {
[ReferenceTracker.CALL]: functionNames.includes("setInterval"),
},
queueMicrotask: {
[ReferenceTracker.CALL]: functionNames.includes("queueMicrotask"),
},
})) {
yield {
node: node as TSESTree.CallExpression,
name: path[path.length - 1],
}
})
}
}
25 changes: 21 additions & 4 deletions src/rules/valid-context-access.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createRule } from "../utils"
import { extractContextReferences } from "./reference-helpers/svelte-context"
import { extractSvelteLifeCycleReferences } from "./reference-helpers/svelte-lifecycle"
import { extractTaskReferences } from "./reference-helpers/microtask"
import type { AST } from "svelte-eslint-parser"
import type { TSESTree } from "@typescript-eslint/types"

@@ -29,6 +30,7 @@ export default createRule("valid-context-access", {
const lifeCycleReferences = Array.from(
extractSvelteLifeCycleReferences(context),
).map((r) => r.node)
const taskReferences = Array.from(extractTaskReferences(context))

// Extract <script> blocks that is not module=context.
const scriptNotModuleElements: AST.SvelteScriptElement[] =
@@ -80,8 +82,8 @@ export default createRule("valid-context-access", {
return []
}

/** Return true if tnodeA is inside of nodeB. */
function isInsideOfNodeB(
/** Return true if tnodeB is inside of nodeA. */
function isInsideOf(
nodeA: TSESTree.Node | AST.SvelteScriptElement,
nodeB: TSESTree.Node,
) {
@@ -93,7 +95,7 @@ export default createRule("valid-context-access", {
/** Return true if the node is there inside of <script> block that is not module=context. */
function isInsideOfSvelteScriptElement(node: TSESTree.Node) {
for (const script of scriptNotModuleElements) {
if (isInsideOfNodeB(script, node)) {
if (isInsideOf(script, node)) {
return true
}
}
@@ -111,14 +113,24 @@ export default createRule("valid-context-access", {
function isAfterAwait(node: TSESTree.CallExpression) {
for (const awaitExpression of awaitExpressions) {
const { belongingFunction, node: awaitNode } = awaitExpression
if (isInsideOfNodeB(node, belongingFunction)) {
if (isInsideOf(node, belongingFunction)) {
Copy link
Member

Choose a reason for hiding this comment

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

There seems to be false positives in complex cases such as:

<script>
  import { getContext } from "svelte"

  async function fn() {
    async function foo() {
      await p
    }

    getContext("a")

    await foo()
  }
  fn()
</script>

I don't think we can check the correct function scope by just checking the range.

continue
}
return awaitNode.range[0] <= node.range[0]
}
return false
}

/** Return true if node is inside of task function */
function isInsideTaskReference(node: TSESTree.CallExpression) {
for (const taskReference of taskReferences) {
if (isInsideOf(taskReference.node, node)) {
return true
}
}
return false
}

/** Let's lint! */
function doLint(
visitedCallExpressions: TSESTree.CallExpression[],
@@ -136,6 +148,11 @@ export default createRule("valid-context-access", {
return
}

if (isInsideTaskReference(currentNode)) {
report(contextCallExpression)
return
}

let { parent } = currentNode
while (parent) {
parent = parent.parent
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
- message: Do not call setContext except during component initialization.
line: 3
column: 3
line: 5
column: 5
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<script context="module">
<script>
import { setContext } from "svelte"
setContext("answer", 42)

setTimeout(() => {
setContext("answer", 42)
})
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- message: Do not call setContext except during component initialization.
line: 6
column: 7
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
import { setContext } from "svelte"

const doSomething = () => {
setTimeout(() => {
setContext("answer", 42)
})
}

doSomething()
</script>
4 changes: 4 additions & 0 deletions tests/src/rules/valid-context-access.ts
Original file line number Diff line number Diff line change
@@ -7,6 +7,10 @@ const tester = new RuleTester({
ecmaVersion: 2020,
sourceType: "module",
},
env: {
browser: true,
es2017: true,
},
})

tester.run(