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

fix(nuxt): support custom route name meta with typedPages #21659

Merged

Conversation

ChronicStone
Copy link
Contributor

@ChronicStone ChronicStone commented Jun 20, 2023

πŸ”— Linked issue

#21637

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Resolves #21637 by parsing at build-time definePageMeta on routes & extracting custom route name

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@ChronicStone
Copy link
Contributor Author

ChronicStone commented Jun 21, 2023

Following @danielroe feedback, the route name resolver now uses estree parser to extract value from AST. Performance cost seem to be slightly higher on the tests I've made so far, I need to test with a bigger amount of pages to measure more precisely

@danielroe danielroe changed the title feat(nuxt): support custom route name through definePageMeta for typed router fix(nuxt): support custom route name through definePageMeta for typed router Jun 22, 2023
@ChronicStone
Copy link
Contributor Author

Updated with your latest advice @danielroe. Now AST parsing happens only if definePageMeta is defined, and happen only on the definePageMeta portion of code to minimize AST parsing.

@danielroe
Copy link
Member

Would you mind taking a look at the failing CI?

@ChronicStone
Copy link
Contributor Author

@danielroe Yep, I planned to have a look tomorrow

@ChronicStone
Copy link
Contributor Author

ChronicStone commented Jun 29, 2023

Failing CI issue should be solved, unit tests now work properly with new implementation of generateRoutesFromFiles.

Also new unit test implemented to cover route name override

@ChronicStone
Copy link
Contributor Author

ChronicStone commented Jun 30, 2023

@danielroe Sorry to mention, I know it isn't ideal, but the new error on CI seems to be related to commits from main being merged to local branch.

Considering the mature of the issue, I'm a bit confused on how I sould proceed to fix this.

packages/schema/src/types/config.ts:141: error TS2339: Property 'nitro' does not exist on type 'NitroRuntimeConfig'.

image

@danielroe
Copy link
Member

The CI error is because you haven't added all the dependencies you're using to the package.json dependencies, and they are being inlined in the package output.

@danielroe danielroe changed the title fix(nuxt): support custom route name through definePageMeta for typed router fix(nuxt): support custom route names from meta with typedPages Jul 3, 2023
@danielroe danielroe merged commit fd2b36a into nuxt:main Jul 4, 2023
28 checks passed
@danielroe danielroe changed the title fix(nuxt): support custom route names from meta with typedPages fix(nuxt): support custom route name meta with typedPages Jul 4, 2023
@github-actions github-actions bot mentioned this pull request Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TYPED ROUTER] - Custom route name through definePageMeta are ignored on generated routes types
2 participants