feat: add externalLinkPrefixes support#1674
feat: add externalLinkPrefixes support#1674JounQin wants to merge 1 commit intoweb-infra-dev:mainfrom
Conversation
✅ Deploy Preview for aquamarine-blini-95325f ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| const errorInfos: string[] = []; | ||
| links | ||
| .filter(link => !IGNORE_REGEXP.test(link)) | ||
| .filter(link => !isExternalUrl(link, routeService.externalLinkPrefixes)) |
There was a problem hiding this comment.
There are some differences here. # is missed, #my-anchor should not be considered as externalUrl but can be ignored in dead links check.
| url.startsWith('https://') || | ||
| url.startsWith('mailto:') || | ||
| url.startsWith('tel:') | ||
| /^((https?|mailto|tel):)?\/\//.test(url) || |
There was a problem hiding this comment.
I think this regex is not as same as before, you can see tests failed in CI. And this regex has edge cases like //www.example.com and for long urls, the performance is not very good for long and complex urls.
There was a problem hiding this comment.
I recommend to keep origin methods to make it more readable and easier to understand for simple checks.
There was a problem hiding this comment.
//www.example.com should not be considered as external?
There was a problem hiding this comment.
I just take an example to show the difference.
Sure, current PR is only basic proposal, I'll update rest part after you agree with this approach. |
@SoonIter Do you have any suggestions for approach to solve this issue？ |
|
Another case is maybe we can support dynamic external link patterns? |
not much, I think |
|
I'll change to use |
LGTM |
|
I'd like to close this PR, because I find it would make using https://github.com/search?q=repo%3Aweb-infra-dev%2Frspress%20isExternalUrl&type=code So for external links, we're using a separated component specifically instead now. What do you think? @Timeless0911 @SoonIter |
Good solution. |
3 participants
Summary
Related Issue
close #1673
Checklist
I'd like to add related tests and document if this proposal is accepted.