feat: add externalLinkPrefixes support #1674
Conversation
✅ Deploy Preview for aquamarine-blini-95325f ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -22,7 +20,7 @@ export function checkLinks( | |||
| ) { | |||
| 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 |
Summary
Related Issue
close #1673
Checklist
I'd like to add related tests and document if this proposal is accepted.