feat(eslint-plugin): add no-missing-exports rule (#2612) #3076
Conversation
✅ Deploy Preview for pinia-official canceled.
|
WalkthroughThis pull request introduces a new ESLint plugin package for Pinia that enforces best practices in setup stores. The plugin includes one rule that detects variables declared inside a setup store function but not exported via the return statement, along with a recommended configuration preset and corresponding tests. Changes
Sequence DiagramsequenceDiagram
participant ESLint as ESLint Parser
participant Rule as no-missing-exports Rule
participant AST as AST Visitor
ESLint->>AST: Parse Pinia setup store code
ESLint->>Rule: Register visitor on ReturnStatement
AST->>Rule: Encounter return statement
Rule->>Rule: Extract returned variable names
Rule->>Rule: Find containing function body
Rule->>Rule: Iterate over VariableDeclaration nodes
alt Variable in returned set
Rule->>Rule: ✓ Valid - no report
else Variable not in returned set
Rule->>ESLint: Report problem: "Variable not exported"
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/eslint-plugin-pinia/package.json (1)
1-12: Add essential package metadata fields.The package.json is missing standard fields like
description,keywords,license,repository,author, andbugs. These fields are important for npm discoverability and proper package documentation.Consider adding:
{ "name": "@pinia/eslint-plugin", "version": "0.0.1", + "description": "ESLint plugin for Pinia best practices", + "keywords": ["eslint", "eslint-plugin", "pinia", "vue"], + "license": "MIT", + "repository": { + "type": "git", + "url": "https://github.com/vuejs/pinia.git", + "directory": "packages/eslint-plugin-pinia" + }, "main": "src/index.js", "type": "commonjs",packages/eslint-plugin-pinia/tests/no-missing-exports.test.js (1)
8-29: Expand test coverage for edge cases.The tests cover basic scenarios but miss important edge cases that the rule should handle: destructuring declarations, function declarations, spread operators in return statements, and non-defineStore functions.
Consider adding test cases for:
- Destructuring:
const { x, y } = obj;- Array destructuring:
const [a, b] = arr;- Function declarations:
function helperFn() {}- Spread in return:
return { ...obj, a };- Return of a variable:
return storeObject;- Non-defineStore functions (should not trigger the rule)
These edge cases will help verify the rule's robustness.
packages/eslint-plugin-pinia/src/rules/no-missing-exports.js (1)
32-42: Clarify the function detection logic.The logic finds the nearest enclosing function but doesn't verify it's a setup store function passed to
defineStore. While this might be acceptable for an initial implementation, consider documenting this limitation or enhancing the detection.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/eslint-plugin-pinia/package.json(1 hunks)packages/eslint-plugin-pinia/src/configs/recommended.js(1 hunks)packages/eslint-plugin-pinia/src/index.js(1 hunks)packages/eslint-plugin-pinia/src/rules/no-missing-exports.js(1 hunks)packages/eslint-plugin-pinia/tests/no-missing-exports.test.js(1 hunks)
🔇 Additional comments (4)
packages/eslint-plugin-pinia/src/configs/recommended.js (1)
1-5: LGTM!The recommended configuration correctly enables the
no-missing-exportsrule with a warning level, which is appropriate for a recommended preset.packages/eslint-plugin-pinia/src/index.js (1)
1-9: LGTM!The plugin entry point correctly exports the rules and configs following standard ESLint plugin structure.
packages/eslint-plugin-pinia/tests/no-missing-exports.test.js (1)
1-29: Add test completion confirmation.The test file runs RuleTester but provides no output to confirm success. Add a console log after the test completes.
tester.run("no-missing-exports", rule, { valid: [ ` export const useX = defineStore('x', () => { const a = 1; return { a }; }); ` ], invalid: [ { code: ` export const useX = defineStore('x', () => { const a = 1; const b = 2; return { a }; }); `, errors: [{ message: "Variable 'b' is declared but not exported via return()" }] } ] }); + +console.log("All tests passed!");Likely an incorrect or invalid review comment.
packages/eslint-plugin-pinia/src/rules/no-missing-exports.js (1)
19-57: I was unable to access the repository codebase directly or find specific documentation about this ESLint rule through web search. Here's what I attempted:
- Direct repository access - Clone operations failed
- GitHub CLI access - Unable to retrieve file contents
- Web search - No specific documentation found for the Pinia
no-missing-exportsruleUnable to verify the critical issue due to insufficient access to the codebase. To properly verify this review comment, I would need to:
- Inspect the actual implementation in
packages/eslint-plugin-pinia/src/rules/no-missing-exports.js- Check if defineStore context detection is already implemented
- Review test cases to understand intended behavior
- Verify the false positive scenario described in the comment
The concern raised (rule triggering on all return statements rather than only those in defineStore functions) is architecturally sound and could plausibly cause false positives, but I cannot confirm its current status in the codebase.
| "main": "src/index.js", | ||
| "type": "commonjs", | ||
| "peerDependencies": { | ||
| "eslint": "^8.0.0" |
There was a problem hiding this comment.
🧩 Analysis chain
ESLint 9 release date latest stable version
<web_search_result>
- ESLint v9 initial release date: April 5, 2024. [1]
- Latest stable v9 release (as of Nov 25, 2025): v9.39.1 — released November 3, 2025. [2]
Sources: [1] ESLint version support page; [2] ESLint release notes / release listing.
</web_search_result>
Script executed:
# Check the package.json file to see the full context
cat -n packages/eslint-plugin-pinia/package.jsonRepository: vuejs/pinia
<shell_result>
Repository: vuejs/pinia
Script execution failed with unrecoverable error: [CAPABILITY:RUN_SHELL_SCRIPT] Failed to clone repository vuejs/pinia: Clone operation failed
</shell_result>
pinia eslint-plugin ESLint 9 compatibility support
<web_search_result>
Short answer: There’s no official ESLint‑9-ready release of eslint-plugin-pinia (the npm package v0.1.11 was published ~1 year ago and does not declare ESLint‑9 support). That means using ESLint 9 may cause peer‑dependency conflicts or require manual wiring (Flat config / FlatCompat) until the plugin adds explicit ESLint‑9/flat‑config support. [1][2]
What to do next
- If you need ESLint 9 now: keep ESLint 9 but configure the plugin manually via eslint.config.js (or use ESLint’s FlatCompat/explicit plugin import) as documented in ESLint’s migration notes. [2]
- If you want the simplest path: stay on ESLint 8 until eslint-plugin-pinia publishes explicit ESLint‑9 support or the plugin’s repo/packaged metadata is updated.
- Confirm by checking the plugin’s repo/package.json or its npm page before upgrading; if you want, I can check the plugin’s repo/package.json for the latest peerDependencies now.
Sources:
- eslint-plugin-pinia on npm (package info, last published). [1]
- ESLint v9 / flat‑config migration and plugin compatibility guidance. [2]
</web_search_result>
Script executed:
# Try using GitHub CLI to check recent commits/issues related to ESLint 9
gh repo view vuejs/pinia --json description,url
gh search issues "repo:vuejs/pinia ESLint 9" --limit 5Repository: vuejs/pinia
<shell_result>
Repository: vuejs/pinia
Script execution failed with unrecoverable error: [CAPABILITY:RUN_SHELL_SCRIPT] Failed to clone repository vuejs/pinia: Clone operation failed
</shell_result>
Update peer dependency range to support ESLint 9, or document ESLint 9 incompatibility.
ESLint 9 was released on April 5, 2024, and is now widely adopted (v9.39.1 as of November 2025). However, eslint-plugin-pinia (v0.1.11) does not declare ESLint 9 support. The current peer dependency ^8.0.0 excludes ESLint 9 entirely, which will cause peer-dependency conflicts for users running ESLint 9. Either update the version range to support ESLint 9 if the plugin is compatible, or document the ESLint 8 requirement and provide migration guidance for ESLint 9 users.
🤖 Prompt for AI Agents
In packages/eslint-plugin-pinia/package.json around line 7, the peerDependency
for "eslint" is locked to "^8.0.0" which excludes ESLint 9; either expand the
range to declare compatibility (e.g., change the peerDependency to accept both
v8 and v9 such as "^8 || ^9" or ">=8 <10") after verifying the plugin builds and
tests cleanly under ESLint 9, or explicitly document in README/CHANGELOG that
only ESLint 8 is supported and add migration guidance; if you choose to expand
the range, run the test suite and a local lint run against an ESLint 9 snapshot,
then update package.json, bump the package version, and add a brief note to the
changelog/README stating ESLint 9 compatibility.
| "eslint": "^8.0.0" | ||
| }, | ||
| "scripts": { | ||
| "test": "eslint --rule 'pinia/no-missing-exports: error' tests/*.js && node tests/no-missing-exports.test.js" |
There was a problem hiding this comment.
Test script has multiple issues.
The test script attempts to run eslint --rule 'pinia/no-missing-exports: error' but the plugin isn't installed or loaded yet. ESLint won't recognize pinia/no-missing-exports without proper plugin registration.
Additionally, eslint is missing from devDependencies, which is required to run the test script.
Consider this approach:
"peerDependencies": {
"eslint": "^8.0.0"
},
+ "devDependencies": {
+ "eslint": "^8.0.0"
+ },
"scripts": {
- "test": "eslint --rule 'pinia/no-missing-exports: error' tests/*.js && node tests/no-missing-exports.test.js"
+ "test": "node tests/no-missing-exports.test.js"
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/eslint-plugin-pinia/package.json around line 10, the test script
tries to run eslint with the rule pinia/no-missing-exports but eslint is not
listed in devDependencies and the plugin isn’t being registered for the run; add
eslint to devDependencies and update the test script to invoke the locally
installed eslint (e.g., via npx or node_modules/.bin/eslint) and explicitly load
the plugin when running (use --plugin pinia or ensure
resolve-plugins-relative-to points at this package) so the rule is recognized,
then keep the existing test runner invocation (node
tests/no-missing-exports.test.js).
| if (node.argument && node.argument.properties) { | ||
| for (const prop of node.argument.properties) { | ||
| if (prop.key && prop.key.name) { | ||
| returnedNames.add(prop.key.name); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Major: Missing validation for ObjectExpression and incomplete property handling.
Line 23 checks node.argument.properties but doesn't verify that node.argument.type === 'ObjectExpression'. If someone returns a variable reference (return storeObject), this will fail silently or behave unexpectedly.
Additionally, the property iteration doesn't handle:
- Spread properties:
return { ...obj, a } - Computed property names:
return { [key]: value } - Method definitions:
return { action() {} }
Apply this diff to add ObjectExpression validation:
const returnedNames = new Set();
// read return object properties (return { a, b })
- if (node.argument && node.argument.properties) {
+ if (node.argument && node.argument.type === "ObjectExpression" && node.argument.properties) {
for (const prop of node.argument.properties) {
+ // Handle spread properties
+ if (prop.type === "SpreadElement") {
+ // For spreads, we can't determine exported names statically
+ // Consider all variables as potentially exported to avoid false positives
+ return;
+ }
if (prop.key && prop.key.name) {
returnedNames.add(prop.key.name);
}
}
+ } else if (node.argument && node.argument.type === "Identifier") {
+ // Return of a variable reference - can't determine exports statically
+ return;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/eslint-plugin-pinia/src/rules/no-missing-exports.js around lines 23
to 29, the code assumes node.argument has properties without verifying it's an
ObjectExpression and does not handle spread, computed, or method properties;
update the logic to first check that node.argument && node.argument.type ===
'ObjectExpression' before iterating, then for each property: skip any
SpreadElement, only process properties with type === 'Property', and for those
handle key types by adding identifier keys (prop.key.name) and literal keys
(String(prop.key.value)) to returnedNames while skipping computed keys
(prop.computed === true) and skipping methods (prop.method === true or prop.kind
!== 'init').
| for (const stmt of fn.body.body) { | ||
| if (stmt.type === "VariableDeclaration") { | ||
| for (const decl of stmt.declarations) { | ||
| const name = decl.id.name; | ||
| if (!returnedNames.has(name)) { | ||
| context.report({ | ||
| node: decl, | ||
| message: `Variable '${name}' is declared but not exported via return()`, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Major: Rule only checks variable declarations, ignoring function declarations.
The rule only checks VariableDeclaration statements but ignores FunctionDeclaration and FunctionExpression assigned to variables. Functions are also common exports in Pinia stores.
Example missing case:
defineStore('x', () => {
function myAction() { } // Should be returned but isn't checked
const state = ref(0);
return { state }; // myAction is missing
});Add a check for function declarations:
for (const stmt of fn.body.body) {
if (stmt.type === "VariableDeclaration") {
for (const decl of stmt.declarations) {
if (decl.id.type === "Identifier") {
const name = decl.id.name;
if (!returnedNames.has(name)) {
context.report({
node: decl,
message: `Variable '${name}' is declared but not exported via return()`,
});
}
}
}
}
+ else if (stmt.type === "FunctionDeclaration" && stmt.id) {
+ const name = stmt.id.name;
+ if (!returnedNames.has(name)) {
+ context.report({
+ node: stmt,
+ message: `Function '${name}' is declared but not exported via return()`,
+ });
+ }
+ }
}🤖 Prompt for AI Agents
In packages/eslint-plugin-pinia/src/rules/no-missing-exports.js around lines
44–56, the loop only inspects VariableDeclaration nodes and ignores
FunctionDeclaration nodes (and function expressions assigned to variables should
be considered exports too); update the loop to also handle stmt.type ===
"FunctionDeclaration" by extracting stmt.id.name and reporting it if not in
returnedNames, and ensure variable declarations that have
function/arrow-function initializers are still checked (you can keep the
existing VariableDeclaration branch but treat declarators with init of type
FunctionExpression/ArrowFunctionExpression the same as other declarators), so
all function declarations/assignments inside the store are validated against
returnedNames.
| for (const stmt of fn.body.body) { | ||
| if (stmt.type === "VariableDeclaration") { | ||
| for (const decl of stmt.declarations) { | ||
| const name = decl.id.name; |
There was a problem hiding this comment.
Critical: Accessing decl.id.name will crash on destructuring patterns.
Line 47 assumes decl.id is an Identifier, but it could be an ArrayPattern (e.g., const [a, b] = arr) or ObjectPattern (e.g., const { x, y } = obj). This will cause a runtime error.
Apply this diff to add a guard:
if (stmt.type === "VariableDeclaration") {
for (const decl of stmt.declarations) {
- const name = decl.id.name;
- if (!returnedNames.has(name)) {
- context.report({
- node: decl,
- message: `Variable '${name}' is declared but not exported via return()`,
- });
- }
+ if (decl.id.type === "Identifier") {
+ const name = decl.id.name;
+ if (!returnedNames.has(name)) {
+ context.report({
+ node: decl,
+ message: `Variable '${name}' is declared but not exported via return()`,
+ });
+ }
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const name = decl.id.name; | |
| if (stmt.type === "VariableDeclaration") { | |
| for (const decl of stmt.declarations) { | |
| if (decl.id.type === "Identifier") { | |
| const name = decl.id.name; | |
| if (!returnedNames.has(name)) { | |
| context.report({ | |
| node: decl, | |
| message: `Variable '${name}' is declared but not exported via return()`, | |
| }); | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/eslint-plugin-pinia/src/rules/no-missing-exports.js around line 47,
the code uses decl.id.name assuming decl.id is an Identifier which will crash
for ArrayPattern or ObjectPattern; guard against non-Identifier nodes by
checking decl.id && decl.id.type === 'Identifier' before accessing .name, and
for other pattern types either skip processing those declarations or extract
their identifiers safely (e.g., traverse the pattern to collect names) depending
on intended behavior; update the logic to only read .name when the check passes
to avoid runtime errors.
|
Hi! 👋 |
|
Hi maintainers 👋 |
1 participant
This PR adds the initial rule for the new Pinia ESLint plugin, as requested in issue #2612.
✔ Added
no-missing-exports✔ Why
According to Pinia docs, all used variables inside a setup store must be returned.
This rule helps developers avoid forgetting to export state/actions/getters.
Closes #2612.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.