feat(eslint-config): add no-page-meta-runtime-values#641
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a new ESLint rule Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 1
🤖 Fix all issues with AI agents
In `@packages/eslint-plugin/src/rules/no-page-meta-runtime-values/index.ts`:
- Around line 17-51: The CONTEXT_APIS set used to mark APIs that require an
active component instance is missing 'provide'; add the string 'provide' to the
existing Set (alongside 'inject' and other Vue APIs) in the CONTEXT_APIS
declaration so that provide is treated as a runtime-only/context API by the
no-page-meta-runtime-values rule.
🧹 Nitpick comments (2)
packages/eslint-config/src/configs/nuxt.ts (1)
53-55: Nit: pages glob is duplicated fromfileSingleRoot(Line 14).The expression
dirs.pages?.map(pagesDir => join(pagesDir, \**/*.${GLOB_EXTS}`)) || []` appears identically in both places. You could extract it into a shared variable to avoid the repetition.Possible refactor
+ const filePages = (dirs.pages?.map(pagesDir => join(pagesDir, `**/*.${GLOB_EXTS}`)) || []).sort() + const fileSingleRoot = [ ...(dirs.layouts?.map(layoutsDir => join(layoutsDir, `**/*.${GLOB_EXTS}`)) || []), - ...(dirs.pages?.map(pagesDir => join(pagesDir, `**/*.${GLOB_EXTS}`)) || []), + ...filePages, ...(dirs.components?.map(componentsDir => join(componentsDir, `**/*.server.${GLOB_EXTS}`)) || []), ].sort()- const filePages = [ - ...(dirs.pages?.map(pagesDir => join(pagesDir, `**/*.${GLOB_EXTS}`)) || []), - ].sort() - if (filePages.length) {packages/eslint-plugin/test/no-page-meta-runtime-values.test.ts (1)
66-137: Consider adding a valid test for object method shorthand.The valid cases cover arrow functions and
functionexpressions, but not the common method shorthand syntax. Since method shorthand desugars to aFunctionExpression, the rule should handle it, but an explicit test documents that intent.// Composable inside method shorthand is fine (depth >= 1) { code: ` definePageMeta({ middleware() { const route = useRoute() return route.path === '/' } }) `, },
| const CONTEXT_APIS = new Set([ | ||
| // Vue reactivity (require active effect scope) | ||
| 'ref', | ||
| 'shallowRef', | ||
| 'customRef', | ||
| 'computed', | ||
| 'reactive', | ||
| 'shallowReactive', | ||
| 'readonly', | ||
| 'shallowReadonly', | ||
| 'toRef', | ||
| 'toRefs', | ||
| 'watch', | ||
| 'watchEffect', | ||
| 'watchPostEffect', | ||
| 'watchSyncEffect', | ||
| 'effectScope', | ||
| 'onScopeDispose', | ||
| // Vue lifecycle hooks | ||
| 'onBeforeMount', | ||
| 'onMounted', | ||
| 'onBeforeUpdate', | ||
| 'onUpdated', | ||
| 'onBeforeUnmount', | ||
| 'onUnmounted', | ||
| 'onActivated', | ||
| 'onDeactivated', | ||
| 'onErrorCaptured', | ||
| 'onRenderTracked', | ||
| 'onRenderTriggered', | ||
| 'onServerPrefetch', | ||
| // Vue component context | ||
| 'inject', | ||
| 'getCurrentInstance', | ||
| ]) |
There was a problem hiding this comment.
provide is missing from CONTEXT_APIS.
provide (counterpart to inject) also requires an active component instance — Vue docs state it can only be called during setup(). Since inject is included on Line 49, provide should be here too for completeness.
Suggested fix
// Vue component context
'inject',
+ 'provide',
'getCurrentInstance',📝 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 CONTEXT_APIS = new Set([ | |
| // Vue reactivity (require active effect scope) | |
| 'ref', | |
| 'shallowRef', | |
| 'customRef', | |
| 'computed', | |
| 'reactive', | |
| 'shallowReactive', | |
| 'readonly', | |
| 'shallowReadonly', | |
| 'toRef', | |
| 'toRefs', | |
| 'watch', | |
| 'watchEffect', | |
| 'watchPostEffect', | |
| 'watchSyncEffect', | |
| 'effectScope', | |
| 'onScopeDispose', | |
| // Vue lifecycle hooks | |
| 'onBeforeMount', | |
| 'onMounted', | |
| 'onBeforeUpdate', | |
| 'onUpdated', | |
| 'onBeforeUnmount', | |
| 'onUnmounted', | |
| 'onActivated', | |
| 'onDeactivated', | |
| 'onErrorCaptured', | |
| 'onRenderTracked', | |
| 'onRenderTriggered', | |
| 'onServerPrefetch', | |
| // Vue component context | |
| 'inject', | |
| 'getCurrentInstance', | |
| ]) | |
| const CONTEXT_APIS = new Set([ | |
| // Vue reactivity (require active effect scope) | |
| 'ref', | |
| 'shallowRef', | |
| 'customRef', | |
| 'computed', | |
| 'reactive', | |
| 'shallowReactive', | |
| 'readonly', | |
| 'shallowReadonly', | |
| 'toRef', | |
| 'toRefs', | |
| 'watch', | |
| 'watchEffect', | |
| 'watchPostEffect', | |
| 'watchSyncEffect', | |
| 'effectScope', | |
| 'onScopeDispose', | |
| // Vue lifecycle hooks | |
| 'onBeforeMount', | |
| 'onMounted', | |
| 'onBeforeUpdate', | |
| 'onUpdated', | |
| 'onBeforeUnmount', | |
| 'onUnmounted', | |
| 'onActivated', | |
| 'onDeactivated', | |
| 'onErrorCaptured', | |
| 'onRenderTracked', | |
| 'onRenderTriggered', | |
| 'onServerPrefetch', | |
| // Vue component context | |
| 'inject', | |
| 'provide', | |
| 'getCurrentInstance', | |
| ]) |
🤖 Prompt for AI Agents
In `@packages/eslint-plugin/src/rules/no-page-meta-runtime-values/index.ts` around
lines 17 - 51, The CONTEXT_APIS set used to mark APIs that require an active
component instance is missing 'provide'; add the string 'provide' to the
existing Set (alongside 'inject' and other Vue APIs) in the CONTEXT_APIS
declaration so that provide is treated as a runtime-only/context API by the
no-page-meta-runtime-values rule.
Deploying nuxt-eslint with
|
| Latest commit: |
9aa30bb
|
| Status: | 🚫 Build failed. |
2 participants
🔗 Linked issue
❓ Type of change
📚 Description
this adds a new eslint rule to help catch some edge cases with
definePageMeta. we can probably extend this further, and I'm a bit wary about hard codinguse*as something to avoid but 🤷