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: warn using storeToRefs inside a store #64

Merged
merged 8 commits into from
Aug 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export default [
| [never-export-initialized-store](docs/rules/never-export-initialized-store.md) | Never export an initialized named or default store. | ✅ ![badge-recommended-flat][] | 🌐 ![badge-all-flat][] | |
| [no-duplicate-store-ids](docs/rules/no-duplicate-store-ids.md) | Disallow duplicate store ids. | ✅ ![badge-recommended-flat][] | 🌐 ![badge-all-flat][] | |
| [no-return-global-properties](docs/rules/no-return-global-properties.md) | Disallows returning globally provided properties from Pinia stores. | ✅ ![badge-recommended-flat][] | 🌐 ![badge-all-flat][] | |
| [no-store-to-refs-in-store](docs/rules/no-store-to-refs-in-store.md) | Disallow use of storeToRefs inside defineStore | ✅ ![badge-recommended-flat][] | 🌐 ![badge-all-flat][] | |
| [prefer-single-store-per-file](docs/rules/prefer-single-store-per-file.md) | Encourages defining each store in a separate file. | | | 🌐 ![badge-all-flat][] |
| [prefer-use-store-naming-convention](docs/rules/prefer-use-store-naming-convention.md) | Enforces the convention of naming stores with the prefix `use` followed by the store name. | | 🌐 ✅ ![badge-all-flat][] ![badge-recommended-flat][] | |
| [require-setup-store-properties-export](docs/rules/require-setup-store-properties-export.md) | In setup stores all state properties must be exported. | ✅ ![badge-recommended-flat][] | 🌐 ![badge-all-flat][] | |
Expand Down
52 changes: 52 additions & 0 deletions docs/rules/no-store-to-refs-in-store.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Disallow use of storeToRefs inside defineStore (`pinia/no-store-to-refs-in-store`)

💼⚠️ This rule is enabled in the following configs: ✅ `recommended`, `recommended-flat`. This rule _warns_ in the following configs: 🌐 `all`, `all-flat`.

<!-- end auto-generated rule header -->

## Rule Details

When stores are cross used, whichever store gets its use... called first will exists as a placeholder in the other store until its own setup function returns. That's why storeToRefs() do not work there and should be avoided altogether with cross used stores.

❌ Examples of **incorrect** code for this rule:

```js
import { useUserStore } from './user'

export const useCartStore = defineStore('cart', () => {
const { user } = storeToRefs(useUserStore())
const list = ref([])

const summary = computed(() => {
return `Hi ${user.name}, you have ${list.value.length} items in your cart. It costs ${price.value}.`
})

function purchase() {
return apiPurchase(user.id, this.list)
}

return { summary, purchase }
})
```

✅ Examples of **correct** code for this rule:

```js
import { useUserStore } from './user'

export const useCartStore = defineStore('cart', () => {
const { user } = useUserStore()
const list = ref([])

const summary = computed(() => {
return `Hi ${user.name}, you have ${list.value.length} items in your cart. It costs ${price.value}.`
})

function purchase() {
return apiPurchase(user.id, this.list)
}

return { summary, purchase }
})

```
22 changes: 14 additions & 8 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,36 @@ import { RULE_NAME as preferNamingConventionName } from './rules/prefer-use-stor
import { RULE_NAME as preferSingleStoreName } from './rules/prefer-single-store-per-file'
import { RULE_NAME as noReturnGlobalPropertiesName } from './rules/no-return-global-properties'
import { RULE_NAME as noDuplicateStoreIdsName } from './rules/no-duplicate-store-ids'
import { RULE_NAME as noStoreToRefs } from './rules/no-store-to-refs-in-store'
import rules from './rules/index'

const plugin = {
rules
}

const allRules = {
[requireSetupStorePropsName]: 'warn',
[neverExportInitializedStoreName]: 'warn',
[noDuplicateStoreIdsName]: 'warn',
[noReturnGlobalPropertiesName]: 'warn',
[noStoreToRefs]: 'warn',
[preferNamingConventionName]: 'warn',
[preferSingleStoreName]: 'off',
[noReturnGlobalPropertiesName]: 'warn',
[noDuplicateStoreIdsName]: 'warn'
[requireSetupStorePropsName]: 'warn'
}

const recommended = {
[requireSetupStorePropsName]: 'error',
[noReturnGlobalPropertiesName]: 'error',
[noDuplicateStoreIdsName]: 'error',
[neverExportInitializedStoreName]: 'error',
[preferNamingConventionName]: 'warn'
[noDuplicateStoreIdsName]: 'error',
[noReturnGlobalPropertiesName]: 'error',
[noStoreToRefs]: 'error',
[preferNamingConventionName]: 'warn',
[requireSetupStorePropsName]: 'error'
}

function createConfig<T extends Record<string, any>>(_rules: T, flat = false) {
function createConfig<T extends Record<string, unknown>>(
_rules: T,
flat = false
) {
const name = 'pinia'
const constructedRules: Record<`pinia/${string}`, string> = Object.keys(
_rules
Expand Down
2 changes: 2 additions & 0 deletions src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import neverExportInitializedStore, { RULE_NAME as neverExportInitializedStoreName } from './never-export-initialized-store'
import noDuplicateStoreIds, { RULE_NAME as noDuplicateStoreIdsName } from './no-duplicate-store-ids'
import noReturnGlobalProperties, { RULE_NAME as noReturnGlobalPropertiesName } from './no-return-global-properties'
import noStoreToRefsInStore, { RULE_NAME as noStoreToRefsInStoreName } from './no-store-to-refs-in-store'
import preferSingleStorePerFile, { RULE_NAME as preferSingleStorePerFileName } from './prefer-single-store-per-file'
import preferUseStoreNamingConvention, { RULE_NAME as preferUseStoreNamingConventionName } from './prefer-use-store-naming-convention'
import requireSetupStorePropertiesExport, { RULE_NAME as requireSetupStorePropertiesExportName } from './require-setup-store-properties-export'
Expand All @@ -10,6 +11,7 @@ export default {
[neverExportInitializedStoreName]: neverExportInitializedStore,
[noDuplicateStoreIdsName]: noDuplicateStoreIds,
[noReturnGlobalPropertiesName]: noReturnGlobalProperties,
[noStoreToRefsInStoreName]: noStoreToRefsInStore,
[preferSingleStorePerFileName]: preferSingleStorePerFile,
[preferUseStoreNamingConventionName]: preferUseStoreNamingConvention,
[requireSetupStorePropertiesExportName]: requireSetupStorePropertiesExport,
Expand Down
2 changes: 1 addition & 1 deletion src/rules/no-duplicate-store-ids.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { resolve } from 'path'
import { AST_NODE_TYPES } from '@typescript-eslint/utils'
import { createEslintRule } from '../utils/rule-creator'
import { resolve } from 'path'

export const RULE_NAME = 'no-duplicate-store-ids'
export type MESSAGE_IDS = 'duplicatedStoreIds'
Expand Down
62 changes: 62 additions & 0 deletions src/rules/no-store-to-refs-in-store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { createEslintRule } from '../utils/rule-creator'

export const RULE_NAME = 'no-store-to-refs-in-store'
export type MESSAGE_IDS = 'storeToRefs'
type Options = []

export default createEslintRule<Options, MESSAGE_IDS>({
name: RULE_NAME,
meta: {
type: 'problem',
docs: {
description: 'Disallow use of storeToRefs inside defineStore'
},
schema: [],
messages: {
storeToRefs:
'Do not use storeToRefs in other stores. Use the store as a whole directly.'
}
},
defaultOptions: [],
create: (context) => {
return {
CallExpression(node) {
if (
node.callee.type === 'Identifier' &&
node.callee.name === 'defineStore' &&
node.arguments.length >= 2
) {
const functionBody = node.arguments[1]
if (
functionBody.type === 'ArrowFunctionExpression' ||
functionBody.type === 'FunctionExpression'
) {
const body = functionBody.body
if (body.type === 'BlockStatement') {
body.body.forEach((statement) => {
if (
statement.type === 'VariableDeclaration' &&
statement.declarations.length > 0
) {
statement.declarations.forEach((declaration) => {
if (
declaration.init &&
declaration.init.type === 'CallExpression' &&
declaration.init.callee.type === 'Identifier' &&
declaration.init.callee.name === 'storeToRefs'
) {
context.report({
node: declaration.init.callee,
messageId: 'storeToRefs'
})
}
})
}
})
}
}
}
}
}
}
})
2 changes: 1 addition & 1 deletion src/rules/require-setup-store-properties-export.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'
import { isIdentifier } from '@typescript-eslint/utils/ast-utils'
import { createEslintRule } from '../utils/rule-creator'
import { getPropertyName, isIdentifier } from '@typescript-eslint/utils/ast-utils'
import { isRefOrReactiveCall } from '../utils/ast-utils'

export const RULE_NAME = 'require-setup-store-properties-export'
Expand Down
20 changes: 10 additions & 10 deletions src/utils/ast-utils.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { AST_NODE_TYPES, TSESTree } from "@typescript-eslint/utils";
export function isRefOrReactiveCall(node: TSESTree.Expression | null): node is TSESTree.CallExpression {
return (
!!node &&
node.type === AST_NODE_TYPES.CallExpression &&
node.callee.type === 'Identifier' &&
(node.callee.name === 'ref' || node.callee.name === 'reactive')
)
}
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'

export function isRefOrReactiveCall(node: TSESTree.Expression | null): node is TSESTree.CallExpression {
return (
!!node &&
node.type === AST_NODE_TYPES.CallExpression &&
node.callee.type === 'Identifier' &&
(node.callee.name === 'ref' || node.callee.name === 'reactive')
)
}
2 changes: 1 addition & 1 deletion tests/fixtures/fixtures.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ const target = resolve(__dirname, '..', 'fixtures')
it.concurrent('fixtures/no-duplicate-store-ids', { fails: true }, async () => {
await execa('npx', ['eslint', 'no-duplicate-store-ids'], {
cwd: target,
stdio: 'pipe',
stdio: 'pipe'
})
})
42 changes: 42 additions & 0 deletions tests/rules/no-store-to-refs-in-store.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import rule, { RULE_NAME } from '../../src/rules/no-store-to-refs-in-store'
import { ruleTester } from '../rule-tester'

ruleTester.run(RULE_NAME, rule, {
valid: [
{
code: `import { defineStore } from 'pinia'
export const useCounterStore = defineStore()`
},
{
code: `import { defineStore } from 'pinia'
export const useCounterStore = defineStore('counter', () => {
const count = ref(0)
return { count }
})
export const useTodoStore = defineStore('todo', () => {
const todo = ref(0)
return { todo }
})`
}
],
invalid: [
{
code: `import { defineStore } from 'pinia'

const useCounterStore = defineStore('counter', () => {
const { user } = storeToRefs(useUserStore())

const count = ref(0)
return { count }
})

const useCounter2Store = defineStore('counter', () => {
})`,
errors: [
{
messageId: 'storeToRefs'
}
]
}
]
})