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.

Sign up for GitHub

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

Jump to bottom

normalizeUrl bug leads to routeBasePath issues #3374

Closed
slorber opened this issue Aug 31, 2020 · 2 comments · Fixed by #3427
Closed

normalizeUrl bug leads to routeBasePath issues #3374

slorber opened this issue Aug 31, 2020 · 2 comments · Fixed by #3427
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: intermediate Issues that are medium difficulty level, e.g. moderate refactoring with a clear test plan. good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. help wanted Asking for outside help and/or contributions to this particular issue or PR.

Comments

@slorber
Copy link
Collaborator

slorber commented Aug 31, 2020 •
edited
Loading

🐛 Bug Report

The "normalizeUrl" pure utility function looks like to it has a bug.

https://github.com/facebook/docusaurus/blob/master/packages/docusaurus-utils/src/index.ts#L281

Tried to add the following test cases:

// ok
      {
        input: ['/'],
        output: '/',
      },

// fail      
      {
        input: ['', '/'],
        output: '/',
      },
      {
        input: ['/', ''],
        output: '/',
      },

And the 2 and 3 test cases failing seems unexpected to me

I think this is the reason why blog/docs option routeBasePath: '' fail, while routeBasePath: '/' works.

I'm going to disallow routeBasePath: '' as a fix for now (#3375).

But still think we should solve the normalizeUrl issue even if the routeBasePath: '' is fixed.

The function is not very easy to understand, but is totally pure, so if someone wants to try providing a fix that would be very helpful ;)
@slorber slorber added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Aug 31, 2020
@slorber slorber changed the title normalizeUrl bug normalizeUrl bug leads to routeBasePath issues Aug 31, 2020
@slorber slorber added difficulty: intermediate Issues that are medium difficulty level, e.g. moderate refactoring with a clear test plan. good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. help wanted Asking for outside help and/or contributions to this particular issue or PR. and removed status: needs triage This issue has not been triaged by maintainers labels Aug 31, 2020
This was referenced Aug 31, 2020
Closed
Merged
@ayshiff
Copy link
Contributor

ayshiff commented Sep 9, 2020

@slorber Can I grab this one ? 👍

@slorber
Copy link
Collaborator Author

slorber commented Sep 9, 2020

yes @ayshiff thanks ;)

@ayshiff ayshiff mentioned this issue Sep 9, 2020
Merged
@Siemienik Siemienik mentioned this issue Dec 21, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: intermediate Issues that are medium difficulty level, e.g. moderate refactoring with a clear test plan. good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. help wanted Asking for outside help and/or contributions to this particular issue or PR.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(v2): normalizeUrl edge cases ayshiff/Docusaurus
2 participants
@slorber @ayshiff