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: add new strategy prefix_regexp #2927

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

s00d
Copy link
Contributor

@s00d s00d commented Apr 19, 2024

❓ 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 pull request introduces a new routing strategy, prefix_regexp, to the nuxt/i18n module. This strategy is designed to streamline the generation of localized routes by reducing the number of routes needed for applications supporting multiple languages.

Key Advantages:

  1. Simplified Route Management: By consolidating non-default languages into a single route using a regular expression, this strategy significantly reduces the complexity and quantity of routes that need to be managed. This makes the application easier to maintain and debug.

  2. Improved Performance: Fewer routes mean quicker setup times for the router and faster re-rendering, which can lead to improvements in overall application performance, especially for larger projects with many supported locales.

  3. Reduced Bundle Size: Since only two routes are generated (one for the default language and one consolidated route for all other languages), the size of the application bundle is reduced. This minimizes the amount of code that needs to be loaded and parsed by the browser, enhancing load times and efficiency.

  4. Enhanced Flexibility: This strategy offers a flexible approach to handle locales by utilizing regular expressions, which can be adapted to various complex requirements without altering the core routing logic.

The implementation of this strategy provides a significant advantage by not only simplifying the management of routes but also by offering a more efficient, scalable, and performance-optimized approach to handling multilingual routing in Nuxt applications. It ensures a balance between user-friendly URLs and the technical requirements of a multilingual setup.

image

πŸ“ Checklist

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

@BobbieGoede
Copy link
Collaborator

This is an interesting solution to the amount of routes scaling out of control with lots of locales, if there are no obvious drawbacks I think it would be preferable to change the other strategies to use this approach instead of adding it as a separate strategy, what do you think?

@s00d
Copy link
Contributor Author

s00d commented Apr 19, 2024

This is an interesting solution to the amount of routes scaling out of control with lots of locales, if there are no obvious drawbacks I think it would be preferable to change the other strategies to use this approach instead of adding it as a separate strategy, what do you think?

Initially, I considered implementing the approach you suggested, but I had concerns about potential risks that could significantly impact system stability. Therefore, it seems preferable to start with a separate strategy that will not affect the current functionality. This method will allow us to systematically conduct all necessary tests, adequately assess potential problems, and their impact on the system.

Moreover, it should be noted that modifying existing code presents a significant challenge due to its volume. A complete overhaul of the entire library would require substantial time resources, which could slow down the process of introducing new features and improvements. Therefore, for the sake of optimizing the development process and reducing risks, it is advisable to start with a separate and isolated approach that will allow for more controlled integration of new solutions.

Copy link
Collaborator

@BobbieGoede BobbieGoede left a comment

Choose a reason for hiding this comment

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

Just an initial check, skimming through the code I think it looks good!

docs/content/docs/5.v7/3.options-reference.md Outdated Show resolved Hide resolved
docs/content/docs/5.v7/6.strategies.md Outdated Show resolved Hide resolved
@BobbieGoede
Copy link
Collaborator

Moreover, it should be noted that modifying existing code presents a significant challenge due to its volume. A complete overhaul of the entire library would require substantial time resources, which could slow down the process of introducing new features and improvements. Therefore, for the sake of optimizing the development process and reducing risks, it is advisable to start with a separate and isolated approach that will allow for more controlled integration of new solutions.

I understand what you mean, the codebase is quite large and adding functionality is more manageable this way. Originally the routing logic was split between this module and https://github.com/intlify/routing/tree/main/packages/vue-i18n-routing, which is one of the reasons why the routing logic is fragmented.

Since I integrated and (slightly) refactored that logic I'm quite familiar with it, if I have time I'll experiment to see if your approach can be used for the other strategies (perhaps making it opt-in with an experimental.regexRouting config), if this proves to be too involved I'm not opposed to adding the additional strategy as you suggest.

@s00d
Copy link
Contributor Author

s00d commented Apr 19, 2024

Moreover, it should be noted that modifying existing code presents a significant challenge due to its volume. A complete overhaul of the entire library would require substantial time resources, which could slow down the process of introducing new features and improvements. Therefore, for the sake of optimizing the development process and reducing risks, it is advisable to start with a separate and isolated approach that will allow for more controlled integration of new solutions.

I understand what you mean, the codebase is quite large and adding functionality is more manageable this way. Originally the routing logic was split between this module and https://github.com/intlify/routing/tree/main/packages/vue-i18n-routing, which is one of the reasons why the routing logic is fragmented.

Since I integrated and (slightly) refactored that logic I'm quite familiar with it, if I have time I'll experiment to see if your approach can be used for the other strategies (perhaps making it opt-in with an experimental.regexRouting config), if this proves to be too involved I'm not opposed to adding the additional strategy as you suggest.

I've uploaded a version here where I tried to switch the code to the new routing, but I didn't finish it. All the tests failed due to the route changes, and manually checking everything is very difficult.

@s00d
Copy link
Contributor Author

s00d commented Apr 24, 2024

@BobbieGoede I've fixed another part of the functionality in the full-regexp branch, but I'm stuck on custom pages and I don't know how to do it properly.

@s00d
Copy link
Contributor Author

s00d commented Apr 27, 2024

I've solved almost all the problems; there are just the last 8 tests left. However, I can't understand how some of them work, for example, the test
specs/routing/prefix-and-default.spec.ts > localePath > route strategy: prefix_and_default > should be worked.

@s00d s00d mentioned this pull request Apr 27, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants