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

fix(core): fix page redirects comparison #1563

Merged
merged 1 commit into from
May 21, 2024
Merged

fix(core): fix page redirects comparison #1563

merged 1 commit into from
May 21, 2024

Conversation

Mister-Hope
Copy link
Member

No description provided.

@Mister-Hope
Copy link
Member Author

The previous code double encode the pathInfer by mistake while comparing.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 9154210680

Details

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 41.193%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/core/src/app/prepare/prepareRoutes.ts 0 4 0.0%
Totals Coverage Status
Change from base Build 9152817878: 0.05%
Covered Lines: 688
Relevant Lines: 1709

💛 - Coveralls


// add redirect to the set when the redirect could not be normalized & encoded to the page path
if (normalizedPathInferred !== path && encodedPathInferred !== path) {
redirectsSet.add(encodedPathInferred)
Copy link
Member

@meteorlxy meteorlxy May 20, 2024

Choose a reason for hiding this comment

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

Maybe we don't need the set anymore? After several iterations, there's only one possible redirect here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should still compare the both because users might set a /中文.html manually. And this extra compare would not take much time.

Copy link
Member

Choose a reason for hiding this comment

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

我是说,是不是不需要这个 set 了

@meteorlxy meteorlxy changed the title fix(core): fix pathInferred compare logic fix(core): fix page redirects comparison May 21, 2024
@meteorlxy meteorlxy merged commit 90a11d9 into main May 21, 2024
30 checks passed
@meteorlxy meteorlxy deleted the redirect branch May 21, 2024 03:02
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.

None yet

3 participants