Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improves compatibility with resolve #665

Merged
merged 7 commits into from
Jan 4, 2020
Merged

Improves compatibility with resolve #665

merged 7 commits into from
Jan 4, 2020

Conversation

arcanis
Copy link
Member

@arcanis arcanis commented Jan 4, 2020

What's the problem this PR addresses?

The resolve package has a mechanism to allow for outsider resolvers to inject themselves into the resolution process, which is a specific file (named normalize-options.js) that they are allowed to modify as they wish.

Our past implementation was injecting the logic within the PnP hook. While it worked mostly fine, it caused an interesting problem when using resolve from within Jest: Jest runs all code from within a sandbox it manages itself, preventing the PnP hook to patch the result of the require call. As a result, resolve didn't knew anything about PnP when run from within a Jest process (browserify/resolve#199 (comment)).

Additionally, my diff adding support for the patch: protocol caused regressions in the install time (in particular for large packages such as flow-bin which took 6m+ just to be fetched, instead of the previous ~4m).

How did you fix it?

As I seem to be doing a bit too much, this diff contains various changes:

  • The libzip is now compiled without ASSERTIONS=1 SAFE_HEAP=1. After removing those flags, the install time for flow-bin went down to ... 40s 🤯 Don't leave debug flags if you don't need them for real, kids.

  • The normalize-options.js file is now patched through the patch: protocol rather than being replaced at runtime. This way it'll work just fine in Jest, as Jest will run the patched code instead of the original one.

  • Patch files applied with the patch: protocol now support a new file directive called semver exclusivity. The way it works: if the target package doesn't match the specified semver range, the file operation will be dropped. This allow you to make patches targeting different versions of a same package (for example, normalize-options.js isn't available pre-1.9).

@arcanis arcanis merged commit 3e324d5 into master Jan 4, 2020
@arcanis arcanis deleted the mael/patch-resolve branch January 4, 2020 18:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant