Skip to content
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

Support "register" tools in module loader #9285

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

marcins
Copy link
Contributor

@marcins marcins commented Oct 3, 2023

↪️ Pull Request

This change modifies the NodePackageManager to support loading modules when using tools like esbuild-register or ts-node.

Essentially these tools work by modifying Module._extensions to add handlers for additional extensions (like .ts). We now pass this extension list to the resolver instance we create in NodePackageManager, and whenever we create a new Module we check whether the list of extensions has been amended.

This means that when using tools like ts-node or esbuild-register, Parcel plugins can be written in TypeScript.

🚨 Test instructions

Added a new integration test which uses a reporter written in TS, and esbuild-register.

I considered adding an integration test using ts-node, but I did a manual check and verified it also works, and it's pretty much the same as esbuild-register in terms of mechanics so didn't think it was worth the extra dependency.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@marcins
Copy link
Contributor Author

marcins commented Oct 3, 2023

14,212 additions, 12,861 deletions not shown because the diff is too large.

Hmm.. I wonder what happened there.. maybe it's the Atlassian package registry. I'll have a look.

EDIT Fixed it.. for some reason I accidentally used npm to remove a package instead of yarn 🤦

@parcel-benchmark
Copy link

parcel-benchmark commented Oct 3, 2023

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.52s -34.00ms
Cached 273.00ms +21.00ms ⚠️

Cold Bundles

Bundle Size Difference Time Difference
dist/legacy/index.ff03421b.js 1.48kb +0.00b 394.00ms -25.00ms 🚀
dist/legacy/index.e9bb1616.js 1.06kb +0.00b 394.00ms -25.00ms 🚀
dist/modern/index.4a29d309.js 921.00b +0.00b 393.00ms -26.00ms 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/legacy/index.ff03421b.js 1.48kb +0.00b 428.00ms +55.00ms ⚠️
dist/legacy/index.e9bb1616.js 1.06kb +0.00b 428.00ms +56.00ms ⚠️
dist/modern/index.4a29d309.js 921.00b +0.00b 428.00ms +56.00ms ⚠️
dist/legacy/index.html 826.00b +0.00b 428.00ms +28.00ms ⚠️
dist/modern/index.html 749.00b +0.00b 427.00ms +29.00ms ⚠️

React HackerNews ✅

Timings

Description Time Difference
Cold 4.06s -74.00ms
Cached 444.00ms +23.00ms ⚠️

Cold Bundles

Bundle Size Difference Time Difference
dist/index.js 459.11kb +0.00b 1.01s -59.00ms 🚀

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 36.78s -329.00ms
Cached 2.40s -33.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/DatePicker.042aeb21.js 47.46kb +0.00b 6.10s -327.00ms 🚀
dist/DatePicker.dd4c3679.js 24.96kb +0.00b 6.10s -327.00ms 🚀
dist/ResourcedEmojiComponent.9a253c26.js 2.47kb +0.00b 6.10s -327.00ms 🚀
dist/pl.bce591be.js 2.25kb +0.00b 6.10s -328.00ms 🚀
dist/cs.bf42283b.js 2.23kb +0.00b 6.10s -328.00ms 🚀
dist/de.90d5c4fa.js 2.17kb +0.00b 6.10s -328.00ms 🚀
dist/fr.ff5d335f.js 2.13kb +0.00b 6.10s -328.00ms 🚀
dist/es.80bf0476.js 2.12kb +0.00b 6.10s -328.00ms 🚀
dist/hu.223c2cde.js 2.10kb +0.00b 6.10s -328.00ms 🚀
dist/fi.98bb8fa8.js 2.09kb +0.00b 6.10s -328.00ms 🚀
dist/ja.7d4156df.js 2.09kb +0.00b 6.10s -328.00ms 🚀
dist/pt_BR.b9e37d37.js 2.06kb +0.00b 6.10s -328.00ms 🚀
dist/ko.9c6bf469.js 1.98kb +0.00b 6.10s -328.00ms 🚀
dist/it.04edb54a.js 1.97kb +0.00b 6.10s -328.00ms 🚀
dist/nb.9bd6db78.js 1.96kb +0.00b 6.10s -328.00ms 🚀
dist/da.d2d8303e.js 1.95kb +0.00b 6.10s -328.00ms 🚀
dist/nl.c4d12122.js 1.94kb +0.00b 6.10s -328.00ms 🚀
dist/feedback.4b745631.js 1.76kb +0.00b 6.10s -328.00ms 🚀
dist/heading6.e6e03f52.js 1.36kb +0.00b 6.10s -328.00ms 🚀
dist/heading5.d2f94d9d.js 1.23kb +0.00b 6.10s -328.00ms 🚀
dist/expand.c983e90a.js 1.18kb +0.00b 6.10s -327.00ms 🚀
dist/pt_PT.e211e609.js 635.00b +0.00b 6.10s -328.00ms 🚀
dist/et.88ef7cb4.js 633.00b +0.00b 6.10s -328.00ms 🚀
dist/is.5f045a22.js 495.00b +0.00b 6.10s -328.00ms 🚀
dist/ro.8d5b380a.js 482.00b +0.00b 6.10s -328.00ms 🚀
dist/en_GB.4c40e6c6.js 472.00b +0.00b 6.10s -328.00ms 🚀
dist/en.e1d21f6d.js 469.00b +0.00b 6.10s -328.00ms 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/component-lazy.51d1dec9.js 58.94kb +0.00b 6.37s +778.00ms ⚠️
dist/pdfRenderer.01deafa1.js 12.04kb +0.00b 8.67s -3.45s 🚀
dist/ro.8d5b380a.js 482.00b +0.00b 8.68s +2.40s ⚠️

Three.js ✅

Timings

Description Time Difference
Cold 3.06s -2.00ms
Cached 341.00ms +13.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

Copy link
Contributor

@mattcompiles mattcompiles left a comment

Choose a reason for hiding this comment

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

Amazing test case 🤌

@marcins marcins merged commit 356be96 into v2 Oct 3, 2023
16 checks passed
@marcins marcins deleted the mszczepanski/support-register-tools-in-module-loader branch October 3, 2023 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants