-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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(utils): always match exclusion root dirs as complete folder paths #7864
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the deploy preview of this PR
|
@@ -68,7 +69,7 @@ export function createAbsoluteFilePathMatcher( | |||
|
|||
function getRelativeFilePath(absoluteFilePath: string) { | |||
const rootFolder = rootFolders.find((folderPath) => | |||
absoluteFilePath.startsWith(folderPath), | |||
absoluteFilePath.startsWith(addTrailingSlash(folderPath)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure between addTrailingSlash
and addTrailingPathSeparator
. Tentatively went with addTrailingSlash
; no one reported the bug before anyway so it won't be a regression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, never mind: Windows tests tell me that addTrailingPathSeparator
is the right one...
Size Change: 0 B Total Size: 812 kB ℹ️ View Unchanged
|
93945d8
to
225c090
Compare
9f1ac0c
to
ef18207
Compare
[addSuffix(folderPath, '/'), addSuffix(folderPath, '\\')].some((p) => | ||
absoluteFilePath.startsWith(p), | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hacky but this will work for everyone. Remember that a single config may be used to build sites on both Windows and Unix, so we need to cater to 2×2 = 4 cases. I would merge like this.
Pre-flight checklist
Motivation
Issue surfaced in #7862
Test Plan
Test links
Deploy preview: https://deploy-preview-_____--docusaurus-2.netlify.app/
Related issues/PRs