Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

better sanitize modules path #1103

Closed
wants to merge 11 commits into from
Closed

better sanitize modules path #1103

wants to merge 11 commits into from

Conversation

robertsLando
Copy link
Contributor

Fixes #1075

@robertsLando
Copy link
Contributor Author

@JeffJassky Try again and let me know

@@ -1243,7 +1243,8 @@ function payloadFileSync(pointer) {
}

function revertMakingLong(f) {
if (/^\\\\\?\\/.test(f)) return f.slice(4);
const prefixRegex = /^\\\\\?[\\]+/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be prefixRegex could be put in global scope to spare the time to construct the Regexp machinery each time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or I could make it part of the function

@JeffJassky
Copy link

JeffJassky commented Mar 30, 2021

I just checked fresh installs of the latest 4.5.1 on both Mac and Linux and had modules loaded successfully on both. I got it working on local machines as well as for CI/CD with Github actions. As it turns out this PR may not be necessary unless others can reproduce the module path issue I was seeing before.

@robertsLando
Copy link
Contributor Author

@JeffJassky Ok so should I close this PR so?

@JeffJassky
Copy link

It seems that way, yes. If the issue resurfaces I'll add details to #1075.

@robertsLando
Copy link
Contributor Author

IMO we could merge this as it removes a repeated pattern and makes the regex more strics. Thoughts @jesec @hipstersmoothie @erossignon ?

@leerob
Copy link
Member

leerob commented Mar 31, 2021

My preference is to leave this as is because it's not broken. If it does turn out to be an issue, then we could re-open and merge. I like to err on the side of caution.

@leerob leerob closed this Mar 31, 2021
@leerob leerob deleted the fix-native-addon branch March 31, 2021 14:12
@robertsLando robertsLando restored the fix-native-addon branch March 31, 2021 15:19
@robertsLando robertsLando reopened this Mar 31, 2021
@github-actions
Copy link

This pull-request is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this pull-request entirely you can add the no-stale label

@github-actions github-actions bot added the Stale label Sep 23, 2021
@github-actions
Copy link

This pull-request is now closed due to inactivity, you can of course reopen or reference this pull-request if you see fit.

@github-actions github-actions bot closed this Sep 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bundling sharp doesn't work anymore since updating to 4.5.0
4 participants