Skip to content

Commit

Permalink
feat: add svelte/require-store-reactive-access rule (#289)
Browse files Browse the repository at this point in the history
* feat: add `svelte/require-store-reactive-access` rule

* Create warm-eagles-sing.md

* fix: add testcases and fix bugs

* fix: error message

* feat: improves demo

* fix: docs

* fix: shim path

* fix: docs and message

* feat: improve require-store-reactive-access
  • Loading branch information
ota-meshi committed Nov 2, 2022
1 parent ed6e347 commit 2895f16
Show file tree
Hide file tree
Showing 116 changed files with 2,359 additions and 13 deletions.
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

0 comments on commit 2895f16

Please sign in to comment.