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(nuxt): use build plugin to access nuxt route injection #21585

Merged
merged 5 commits into from
Aug 7, 2023

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Jun 15, 2023

πŸ”— Linked issue

resolves #20674

❓ 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

This adds an experimental option to sync $route template object with the Nuxt-managed route.

πŸ“ Checklist

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

@danielroe danielroe requested a review from manniL June 15, 2023 17:45
@danielroe danielroe marked this pull request as ready for review June 15, 2023 17:46
@atinux
Copy link
Member

atinux commented Jun 18, 2023

Not sure that this should be as an experimental feature since it actually give the same behavior without and with vue-router.

Also, I also think that the $route and useRoute() should only changed once the next page is resolved.

I would consider this as a bug fix IMO. Happy to discuss about it if you see it with another POV.

@danielroe
Copy link
Member Author

Agreed. However, personally I would advise against using these kind of injections as much as possible in the template as it makes the 'dependencies' of the component less explicit.

Yet the main issue I have is that I'm not sure of the impact on performance. We might be able instead to transform and auto execute useRoute based on usage. What do you think?

@danielroe danielroe changed the title feat(nuxt): allow configuring a mixin to keep $route injection synced feat(nuxt): use build plugin to access nuxt route injection Aug 2, 2023
@danielroe danielroe requested a review from antfu August 2, 2023 13:15
@danielroe
Copy link
Member Author

@antfu Would love your thoughts on whether it's safe to access _ctx._.provides in this way.

@antfu
Copy link
Member

antfu commented Aug 2, 2023

I am not about the approach of injecting on every Vue file. Instead, maybe we should hijack/proxy the vue-router plugin, to register the global $route with our modified one. Or you have already tried that approach?

https://github.com/vuejs/router/blob/98e9af6adbdd4497bbfa6965ec6cd5741c448007/packages/router/src/router.ts#L1215-L1219

@danielroe
Copy link
Member Author

Oh, sorry - I updated it in the last commit so it doesn't inject in every file but directly accesses the inject in ctx only if the $route injection is accessed in the template.

@danielroe danielroe merged commit 305d6de into main Aug 7, 2023
30 checks passed
@danielroe danielroe deleted the feat/route-sync-mixin branch August 7, 2023 13:19
@danielroe
Copy link
Member Author

@antfu Very up for a different implementation here, but wanted to get this in for 3.7 release.

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.

$route updated too early when switching page
3 participants