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: experimental typed routes #3142

Merged

Conversation

BobbieGoede
Copy link
Collaborator

@BobbieGoede BobbieGoede commented Sep 27, 2024

πŸ”— Linked issue

❓ 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

Important

Very experimental and relies on nuxt/router/uvr internals, this will not be backported to v8.

I'm not very happy with the implementation right now, but it does work... I think the demand/convenience of this feature somewhat justifies adding this albeit under an experimental flag.

We should communicate very clearly that this relies on internals of Nuxt, Vue Router and unplugin-vue-router, perhaps we can make the types fallback to original behavior on failure (maybe it already does). A proper implementation will likely require collaboration with the maintainers of those packages, we should not expect this to keep working as is.

This will need some documentation and basic tests so we know things fail if internals have changed.

Demo stackblitz project here: https://stackblitz.com/edit/bobbiegoede-nuxt-i18n-starter-srohiz?file=nuxt.config.ts&file=composables%2Ftest.ts

Demo of localePath autocompletion

Code_2024-09-27_12-02-50.mp4

Demo of custom routes configuration autocompletion and basic validation (prefix '/')

Code_2024-09-27_12-06-59.mp4

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

@BobbieGoede BobbieGoede self-assigned this Sep 27, 2024
@BobbieGoede BobbieGoede marked this pull request as ready for review September 27, 2024 09:16
@BobbieGoede BobbieGoede changed the title feat: experimental typed routes [DRAFT] feat: experimental typed routes Sep 27, 2024
Copy link

pkg-pr-new bot commented Sep 27, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/@nuxtjs/i18n@3142

commit: 352ce15

@BobbieGoede BobbieGoede marked this pull request as draft September 27, 2024 09:26
@BobbieGoede BobbieGoede marked this pull request as ready for review September 27, 2024 20:47
src/gen.ts Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow! This typing is too amazing!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should reference the original source types πŸ˜… I had to redefine/reimplement a bunch of internal types of vue-router and uvr, so the credits for these go to @posva

Copy link
Collaborator

@kazupon kazupon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just reviewed your PR!
It's an excellent for me!

@kazupon
Copy link
Collaborator

kazupon commented Sep 28, 2024

Since it uses the vue-router type, it would be good if posva is able to review it.

@posva
Can you review it ?, if you have the time πŸ™

@BobbieGoede
Copy link
Collaborator Author

@userquin
If you see some obvious structural improvements, feel free to suggest them, the must be some way to leave out the augmentations in internal-global-types.d.ts πŸ€”

/**
* Helper to generate a type safe version of the {@link RouteLocationAsRelative} type.
*/
export interface RouteLocationAsRelativeTyped<
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the same type exported by the router?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm most of the types should be the same except suffixed with I18n, which this type seems to be missing. This might conflict with existing router types 🫣 going to look into it.

Reasoning behind redefining the same types but suffixed is to use the RouteMapI18n type, perhaps there's a better way to go about this that I'm unaware of.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn’t extending the same route map from Vue router works? Or do you need to have two separate route maps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're generating routes for each locale (depending on the routing strategy) and custom localized route the generated types are localized, this PR generates RouteMapI18n (which ironically contains the routes without localization) to be used for the i18n routing utility functions.

Some examples to illustrate the problem we're trying to solve:

Named route completion
image

Path completion (not desirable when using custom routes)
image

I guess this is specific to i18n, I doubt there are other use cases where generating a separate map is necessary πŸ€”

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would need to try it out to see the needed final types. I was hoping the types were flexible enough to at least keep things typer for the router methods. I18n methods could use extra types for localized route names (that seems to be the main difference)

@BobbieGoede BobbieGoede changed the title [DRAFT] feat: experimental typed routes feat: experimental typed routes Sep 29, 2024
@BobbieGoede BobbieGoede merged commit c103d13 into nuxt-modules:main Sep 30, 2024
9 checks passed
@BobbieGoede BobbieGoede deleted the feat/generated-route-types-2 branch September 30, 2024 11:38
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.

4 participants