-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
fix(nuxt): add TS parenthesis and as expression for page meta extraction #32914
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
Conversation
|
WalkthroughThe Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/nuxt/src/pages/utils.ts (1)
678-680
: Unwrap more TS/JS wrappers to make serialisability checks more robustGood addition. For completeness, consider unwrapping a few more harmless wrappers that often appear around literals:
- TSTypeAssertion (angle-bracket assertions in .ts)
- TSNonNullExpression (postfix ! non-null)
- ChainExpression (parenthesised optional-chaining result)
Adding these keeps behaviour consistent with your new handling of TSAsExpression and ParenthesizedExpression.Proposed minimal change:
- if (node.type === 'TSSatisfiesExpression' || node.type === 'TSAsExpression' || node.type === 'ParenthesizedExpression') { + if ( + node.type === 'TSSatisfiesExpression' || + node.type === 'TSAsExpression' || + node.type === 'TSTypeAssertion' || // e.g. <T>'foo' in .ts (not .tsx) + node.type === 'TSNonNullExpression' || // e.g. ('foo' as const)! + node.type === 'ChainExpression' || // e.g. ('foo' as const)?.toString + node.type === 'ParenthesizedExpression' + ) { return isSerializable(code, node.expression) }Note: Please confirm the exact node type names as emitted by oxc for your parser mode; if
TSTypeAssertion
isn’t emitted in your contexts, you can omit it.packages/nuxt/test/page-metadata.test.ts (2)
156-172
: Great test for TS “as” assertions; add an angle-bracket assertion case tooThis validates TSAsExpression nicely. To cover the other assertion syntax in .ts (not .tsx), add a test for a TSTypeAssertion as well, e.g.
name: <PageName>'name-from-page-meta'
.Example test to add elsewhere in this suite:
it('should extract metadata with TS angle-bracket type assertion', () => { const meta = getRouteMeta(` <script setup lang="ts"> type PageName = 'name-from-page-meta' | 'whatever'; definePageMeta({ name: <PageName>'name-from-page-meta', }); </script> `, filePath) expect(meta).toStrictEqual({ name: 'name-from-page-meta' }) })
174-188
: Nit: align test title wording with AST namingOptional: “TS ParenthesisExpression with as” could be reworded to “TS parenthesised expression with as” (human wording) or “ParenthesizedExpression with as” (AST wording) for consistency with the implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/nuxt/src/pages/utils.ts
(1 hunks)packages/nuxt/test/page-metadata.test.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/pages/utils.ts
packages/nuxt/test/page-metadata.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
packages/nuxt/test/page-metadata.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: Tofandel
PR: nuxt/nuxt#0
File: :0-0
Timestamp: 2024-11-11T12:34:22.648Z
Learning: Ensure that AI-generated summaries accurately reflect the key changes in the PR, focusing on notable changes such as the removal of unused imports and variables starting with underscores.
Learnt from: GalacticHypernova
PR: nuxt/nuxt#29661
File: packages/kit/src/template.ts:227-229
Timestamp: 2024-11-28T21:22:40.496Z
Learning: In `packages/kit/src/template.ts`, when updating the `EXTENSION_RE` regular expression for TypeScript configuration, avoid using patterns like `(\.\w+)+$` as they can result in catastrophic backtracking.
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
PR: nuxt/nuxt#29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
packages/nuxt/src/pages/utils.ts
packages/nuxt/test/page-metadata.test.ts
📚 Learning: 2024-11-28T21:22:40.496Z
Learnt from: GalacticHypernova
PR: nuxt/nuxt#29661
File: packages/kit/src/template.ts:227-229
Timestamp: 2024-11-28T21:22:40.496Z
Learning: In `packages/kit/src/template.ts`, when updating the `EXTENSION_RE` regular expression for TypeScript configuration, avoid using patterns like `(\.\w+)+$` as they can result in catastrophic backtracking.
Applied to files:
packages/nuxt/src/pages/utils.ts
📚 Learning: 2025-07-18T16:46:07.446Z
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T16:46:07.446Z
Learning: Applies to **/*.{ts,tsx} : Follow standard TypeScript conventions and best practices
Applied to files:
packages/nuxt/src/pages/utils.ts
📚 Learning: 2025-07-18T16:46:07.446Z
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T16:46:07.446Z
Learning: Applies to **/e2e/**/*.{ts,js} : Write end-to-end tests using Playwright and `nuxt/test-utils`
Applied to files:
packages/nuxt/test/page-metadata.test.ts
📚 Learning: 2025-07-18T16:46:07.446Z
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T16:46:07.446Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Write unit tests for core functionality using `vitest`
Applied to files:
packages/nuxt/test/page-metadata.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: codeql (javascript-typescript)
- GitHub Check: build
- GitHub Check: code
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #32914 will not alter performanceComparing Summary
|
🔗 Linked issue
#32902 (comment)
📚 Description
this PR handles additionnal TS expressions for page meta extraction