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 possibility to disable default redirect for prefix_and_default strategy #1437

Open
wants to merge 9 commits into
base: v7
Choose a base branch
from

Conversation

Renhor
Copy link

@Renhor Renhor commented May 4, 2022

Regarding to issue #1401

Added property to config:

i18n: {
  disableDefaultRedirect: boolean | string[] // false by default
}

It works only for strategy prefix_and_default.
It disables redirect for routes are prefixed with default language, or we can choose custom path pattern for disabling redirect behaviour:

i18n: {
  disableDefaultRedirect: [
    '/about',
    '/blog/*'
  ]
}

UPD:

Added property to config:

type PrefixAndDefaultRule = 'default' | 'prefix'

i18n: {
  prefixAndDefaultRule?: PrefixAndDefaultRule
}

By setting the value prefixAndDefaultRule = 'prefix' we are no longer redirected from prefixed routes and if there is no prefix then the other routers will also have no prefix.

For now, I decided not to implement an array with a list of routes for which the default behavior is disabled

Copy link
Collaborator

@rchl rchl left a comment

Choose a reason for hiding this comment

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

This would also need documentation to be updated with new option and some tests.

src/templates/plugin.utils.js Outdated Show resolved Hide resolved
src/templates/plugin.main.js Outdated Show resolved Hide resolved
src/templates/utils-common.js Outdated Show resolved Hide resolved
src/templates/utils-common.js Outdated Show resolved Hide resolved
types/index.d.ts Outdated Show resolved Hide resolved
src/templates/plugin.main.js Outdated Show resolved Hide resolved
src/templates/plugin.main.js Outdated Show resolved Hide resolved
@Renhor
Copy link
Author

Renhor commented May 11, 2022

rchl Thanks for the quick review. Unfortunately, I will only be able to continue working on the task today. I decided to abandon the route array and changed the implementation following your advice. Updated the information in the first post

docs/content/en/api.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@rchl rchl left a comment

Choose a reason for hiding this comment

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

I have not looked properly at the changes yet because many tests are failing.

docs/content/en/options-reference.md Outdated Show resolved Hide resolved
@Renhor Renhor force-pushed the feat/disableDefaultRedirect branch from 275aa9f to 3297711 Compare May 12, 2022 11:21
@Renhor
Copy link
Author

Renhor commented May 12, 2022

Corrected the code that was causing the tests to crash

@Renhor Renhor requested a review from rchl May 16, 2022 11:16
@juliomontilla100
Copy link

juliomontilla100 commented Oct 3, 2022

I need this feature ! please, thanks... @Renhor any update bro?

@DanielHefti
Copy link

This feature would be highly appreciated. Get in touch with me, if there is anything I can do.

@ineshbose ineshbose added the v7 label May 13, 2023
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.

5 participants