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

chore!: remove deprecated legacy dynamic route params #2801

Conversation

BobbieGoede
Copy link
Collaborator

@BobbieGoede BobbieGoede commented Feb 16, 2024

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 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)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

The deprecation of using nuxtI18n with definePageMeta was a bit soft, as we kept using the same key internally for setI18nParams I can imagine there may have been users ignoring the warning. I changed the nuxtI18n key to nuxtI18nInternal, this will break the functionality for projects using the deprecated functionality and better communicate that this is an internal key.

At some point we may or may not move away from using the meta key on routes, this is already the case when having experimental.switchLocalePathLinkSSR feature enabled.

πŸ“ Checklist

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

Copy link

nuxt-studio bot commented Feb 16, 2024

βœ… Live Preview ready!

Name Edit Preview Latest Commit
i18n Edit on Studio β†—οΈŽ View Live Preview 3ea37e1

@kazupon
Copy link
Collaborator

kazupon commented Feb 17, 2024

Thank you!

I had planned to drop features in v8.1 that I had marked as deprecated prior to the v8.0 release.
However, nuxt i18n versioning is in semver, and according to semver policy that is equivalent to a major change.
And also, the recent OSS semver for automatic tracking tools like renovate and dependbot, as well as release tools.

So I believe that if we drop deprecation, we should release as v9.

There may be other deprecations in nuxt i18n.
I think we should pickup all of them once and drop them and release v9.

@BobbieGoede BobbieGoede added the v9 label Feb 19, 2024
@BobbieGoede
Copy link
Collaborator Author

However, nuxt i18n versioning is in semver, and according to semver policy that is equivalent to a major change.

There may be other deprecations in nuxt i18n.
I think we should pickup all of them once and drop them and release v9.

I agree, I'll label it v9 and set it to draft. It's good to be careful with breaking changes, I think with some planning it can be combined with some other changes.

Maybe I'll open an issue/discussion for v9 in the coming days to track which issues/changes could be included so users can give feedback or suggest changes.

@BobbieGoede BobbieGoede marked this pull request as draft February 19, 2024 16:20
@kazupon
Copy link
Collaborator

kazupon commented Feb 20, 2024

Maybe I'll open an issue/discussion for v9 in the coming days to track which issues/changes could be included so users can give feedback or suggest changes.

Good idea! πŸ‘
Let's open the issue/discussion!

@kazupon kazupon mentioned this pull request Jun 28, 2024
14 tasks
@BobbieGoede BobbieGoede force-pushed the chore/deprecate-legacy-dynamic-route-params-fully branch from 3ea37e1 to e19819e Compare July 28, 2024 14:31
@BobbieGoede BobbieGoede changed the title chore: deprecate legacy dynamic route params fully chore!: deprecate legacy dynamic route params fully Jul 28, 2024
@BobbieGoede BobbieGoede changed the title chore!: deprecate legacy dynamic route params fully chore!: remove deprecated legacy dynamic route params Jul 28, 2024
@BobbieGoede BobbieGoede self-assigned this Jul 28, 2024
@BobbieGoede BobbieGoede marked this pull request as ready for review July 28, 2024 14:37
@BobbieGoede
Copy link
Collaborator Author

BobbieGoede commented Jul 28, 2024

@kazupon
I have updated this branch and changed the relevant docs in the v9 documentation, originally this PR also added logs whenever the old nuxtI18n meta key were set by users but since we have the deprecation warning in v8 and the added migration docs I don't think those logs are necessary.

Related to this feature, should we also remove experimental.switchLocalePathLinkSSR and have it enabled by default? It would clean up the implementation further and will be easier to document how to get proper dynamic route parameter for SSR projects. This change would be a separate PR.

@BobbieGoede BobbieGoede force-pushed the chore/deprecate-legacy-dynamic-route-params-fully branch from a3c9d1c to c426492 Compare September 13, 2024 20:27
@BobbieGoede BobbieGoede requested review from kazupon and removed request for kazupon September 13, 2024 20:37
@BobbieGoede BobbieGoede force-pushed the chore/deprecate-legacy-dynamic-route-params-fully branch from 34c589a to d2fa9e6 Compare September 14, 2024 22:22
@BobbieGoede BobbieGoede merged commit a00c2f4 into nuxt-modules:main Sep 14, 2024
8 checks passed
@BobbieGoede BobbieGoede deleted the chore/deprecate-legacy-dynamic-route-params-fully branch September 14, 2024 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants