Skip to content

feat: add svelte/require-store-reactive-access rule #289

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

Merged
merged 12 commits into from
Nov 2, 2022
Merged
5 changes: 5 additions & 0 deletions .changeset/warm-eagles-sing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-svelte": patch
---

feat: add `svelte/require-store-reactive-access` rule
1 change: 1 addition & 0 deletions .stylelintignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ LICENSE
# should we ignore markdown files?
*.md
/docs-svelte-kit/
/coverage
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ These rules relate to possible syntax or logic errors in Svelte code:
| [svelte/no-store-async](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-store-async/) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | |
| [svelte/no-unknown-style-directive-property](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-unknown-style-directive-property/) | disallow unknown `style:property` | :star: |
| [svelte/require-store-callbacks-use-set-param](https://ota-meshi.github.io/eslint-plugin-svelte/rules/require-store-callbacks-use-set-param/) | store callbacks must use `set` param | |
| [svelte/require-store-reactive-access](https://ota-meshi.github.io/eslint-plugin-svelte/rules/require-store-reactive-access/) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :wrench: |
| [svelte/valid-compile](https://ota-meshi.github.io/eslint-plugin-svelte/rules/valid-compile/) | disallow warnings when compiling. | :star: |
| [svelte/valid-prop-names-in-kit-pages](https://ota-meshi.github.io/eslint-plugin-svelte/rules/valid-prop-names-in-kit-pages/) | disallow props other than data or errors in Svelte Kit page components. | |

Expand Down
40 changes: 36 additions & 4 deletions docs-svelte-kit/shim/path.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,44 @@ function isAbsolute() {
}

function join(...args) {
return args.join("/")
return args.length ? normalize(args.join("/")) : "."
}

const sep = "/"
function normalize(path) {
let result = []
for (const part of path.replace(/\/+/gu, "/").split("/")) {
if (part === "..") {
if (result[0] && result[0] !== ".." && result[0] !== ".") result.shift()
} else if (part === "." && result.length) {
// noop
} else {
result.unshift(part)
}
}
return result.reverse().join("/")
}

const posix = { dirname, extname, resolve, relative, sep, isAbsolute, join }
const sep = "/"
const posix = {
dirname,
extname,
resolve,
relative,
sep,
isAbsolute,
join,
normalize,
}
posix.posix = posix
export { dirname, extname, posix, resolve, relative, sep, isAbsolute, join }
export {
dirname,
extname,
posix,
resolve,
relative,
sep,
isAbsolute,
join,
normalize,
}
export default posix
48 changes: 46 additions & 2 deletions docs-svelte-kit/src/lib/eslint/scripts/ts-create-program.mts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type typescript from "typescript"
import type tsvfs from "@typescript/vfs"
import path from "path"
type TS = typeof typescript
type TSVFS = typeof tsvfs

Expand Down Expand Up @@ -68,10 +69,53 @@ export async function createVirtualCompilerHost(
true,
ts,
)

// Setup svelte type definition modules
for (const [key, get] of Object.entries(
// @ts-expect-error -- ignore
import.meta.glob("../../../../../node_modules/svelte/**/*.d.ts", {
as: "raw",
}),
)) {
const modulePath = key.slice("../../../../..".length)

fsMap.set(
modulePath,
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore
await (get as any)(),
)
}

const system = tsvfs.createSystem(fsMap)
const host = tsvfs.createVirtualCompilerHost(system, compilerOptions, ts)
// eslint-disable-next-line @typescript-eslint/unbound-method -- backup original
const original = { getSourceFile: host.compilerHost.getSourceFile }
const original = {
// eslint-disable-next-line @typescript-eslint/unbound-method -- backup original
getSourceFile: host.compilerHost.getSourceFile,
}
host.compilerHost.resolveModuleNames = function (
moduleNames,
containingFile,
) {
return moduleNames.map((m) => {
const targetPaths: string[] = []
if (m.startsWith(".")) {
targetPaths.push(path.join(path.dirname(containingFile), m))
} else {
targetPaths.push(`/node_modules/${m}`)
}
for (const modulePath of targetPaths.flatMap((m) => [
`${m}.d.ts`,
`${m}.ts`,
`${m}/index.d.ts`,
`${m}/index.ts`,
])) {
if (fsMap.has(modulePath)) {
return { resolvedFileName: modulePath }
}
}
return undefined
})
}
host.compilerHost.getSourceFile = function (
fileName,
languageVersionOrOptions,
Expand Down
1 change: 1 addition & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ These rules relate to possible syntax or logic errors in Svelte code:
| [svelte/no-store-async](./rules/no-store-async.md) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | |
| [svelte/no-unknown-style-directive-property](./rules/no-unknown-style-directive-property.md) | disallow unknown `style:property` | :star: |
| [svelte/require-store-callbacks-use-set-param](./rules/require-store-callbacks-use-set-param.md) | store callbacks must use `set` param | |
| [svelte/require-store-reactive-access](./rules/require-store-reactive-access.md) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :wrench: |
| [svelte/valid-compile](./rules/valid-compile.md) | disallow warnings when compiling. | :star: |
| [svelte/valid-prop-names-in-kit-pages](./rules/valid-prop-names-in-kit-pages.md) | disallow props other than data or errors in Svelte Kit page components. | |

Expand Down
97 changes: 97 additions & 0 deletions docs/rules/require-store-reactive-access.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
---
pageClass: "rule-details"
sidebarDepth: 0
title: "svelte/require-store-reactive-access"
description: "disallow to use of the store itself as an operand. Need to use $ prefix or get function."
---

# svelte/require-store-reactive-access

> disallow to use of the store itself as an operand. Need to use $ prefix or get function.

- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

## :book: Rule Details

This rule disallow to use of the store itself as an operand.
You should access the store value using the `$` prefix or the `get` function.

<ESLintCodeBlock fix>

<!--eslint-skip-->

```svelte
<script>
/* eslint svelte/require-store-reactive-access: "error" */
import { writable, get } from "svelte/store"
const storeValue = writable("world")
const color = writable("red")

/* ✓ GOOD */
$: message = `Hello ${$storeValue}`

/* ✗ BAD */
$: message = `Hello ${storeValue}`
</script>

<!-- ✓ GOOD -->
<p>{$storeValue}</p>
<p>{get(storeValue)}</p>

<p class={$storeValue} />
<p style:color={$color} />

<MyComponent prop="Hello {$storeValue}" />
<MyComponent bind:this={$storeValue} />
<MyComponent --style-props={$storeValue} />
<MyComponent {...$storeValue} />

<!-- ✗ BAD -->
<p>{storeValue}</p>

<p class={storeValue} />
<p style:color />

<MyComponent prop="Hello {storeValue}" />
<MyComponent bind:this={storeValue} />
<MyComponent --style-props={storeValue} />
<MyComponent {...storeValue} />
```

</ESLintCodeBlock>

This rule checks the usage of store variables only if the store can be determined within a single file.
However, when using `@typescript-eslint/parser` and full type information, this rule uses the type information to determine if the expression is a store.

<!--eslint-skip-->

```ts
// fileName: my-stores.ts
import { writable } from "svelte/store"
export const storeValue = writable("hello")
```

<!--eslint-skip-->

```svelte
<script lang="ts">
/* eslint svelte/require-store-reactive-access: "error" */
import { storeValue } from "./my-stores"
</script>

<!-- ✓ GOOD -->
<p>{$storeValue}</p>

<!-- ✗ BAD -->
<p>{storeValue}</p>
```

## :wrench: Options

Nothing.

## :mag: Implementation

- [Rule source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/src/rules/require-store-reactive-access.ts)
- [Test source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/tests/src/rules/require-store-reactive-access.ts)
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@
"stylus": "^0.59.0",
"svelte": "^3.46.1",
"svelte-adapter-ghpages": "0.0.2",
"svelte-i18n": "^3.4.0",
"type-coverage": "^2.22.0",
"typescript": "^4.5.2",
"vite": "^3.1.0-0",
Expand All @@ -174,7 +175,7 @@
"access": "public"
},
"typeCoverage": {
"atLeast": 98.69,
"atLeast": 98.72,
"cache": true,
"detail": true,
"ignoreAsAssertion": true,
Expand Down
7 changes: 1 addition & 6 deletions src/rules/indent-helpers/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
isSemicolonToken,
} from "eslint-utils"
import type { ESNodeListener } from "../../types-for-node"
import { getParent } from "../../utils/ast-utils"

type NodeListener = ESNodeListener

Expand Down Expand Up @@ -1097,12 +1098,6 @@ export function defineVisitor(context: IndentContext): NodeListener {
}
}

/** Get the parent node from the given node */
function getParent(node: ESTree.Node): ESTree.Node | null {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore
return (node as any).parent || null
}

/**
* Checks whether given text is known button type
*/
Expand Down
Loading