fix(v2): make correct internal link check #2424
Conversation
|
Deploy preview for docusaurus-2 ready! Built with commit 18351c8 |
|
|
||
| describe('isInternalUrl', () => { | ||
| test('should be true for root relative links', () => { | ||
| expect(isInternalUrl('/foo/bar')).toBeTruthy(); |
There was a problem hiding this comment.
I think this might be incorrect in the cases it is a relative link from the root of the domain. For example your site is a www.example.com/website/ and your path is /hello/foo/ which is intended to lead to www.example.com/hello/foo. Maybe we need to compare with the baseUrl to decide if it's an internal link? WDYT?
There was a problem hiding this comment.
I thought about it, but how do we get the base url in this util function? Pass baseUrl as an argument? Hmm, I don’t know, it seems to me then users need to only use relative links for this.
There was a problem hiding this comment.
We can read the baseUrl from the config, so users don't have to pass it in. Maybe check if the string values starts with baseUrl? Not sure too.
There was a problem hiding this comment.
I would prefer to leave it as it is, if in the future users encounter such a case, then we will try to solve it.
Motivation
Make fixes as per comment #2420 (comment) from @38elements.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
See tests.
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)