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

Fix: #4695 Reference CommonJS child bundles with a require call #4744

Merged
merged 12 commits into from
Jul 13, 2020

Conversation

jfrconley
Copy link
Contributor

↪️ Pull Request

This pull request adds support to JSRuntime for referencing CommonJS child bundles with a require call. No handling of this was present, so the current behavior was to use a URL reference.

Resolves: #4695

💻 Examples

There isn't an easy way to see this in action without a custom bundler. I created a simple one (https://github.com/jfrconley/parcel-bundler-splitable) that allows listing files you want to split the bundle on. The integration test relies on a custom bundler with a hardcoded path to split on.

🚨 Test instructions

Testing is covered by added integration test: commonjs-bundle-require

✔️ 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

@parcel-bundler parcel-bundler deleted a comment from height bot Jun 16, 2020
@jfrconley
Copy link
Contributor Author

@mischnic Is there a timeline for when this can be merged? Keeping a custom runtime plugin up to date is causing some issues for us.

@mischnic
Copy link
Member

mischnic commented Jul 6, 2020

I'll take a look.
Could you slim down the bundler plugin you added in the test so that it only includes the bare minimum to make the test work? For example the optimize step likely doesn't do anything here

@jfrconley
Copy link
Contributor Author

I removed optimize and deduplicate. Additionally, I've resolved merge conflicts and pulled in upstream changes

@jfrconley
Copy link
Contributor Author

@mischnic Is there any other cleanup I need to do before this can be merged?

@mischnic
Copy link
Member

mischnic commented Jul 13, 2020

I would have suggested supporting ESM as well while we're at it, but that can't work yet for other reasons...

…le-require/node_modules/parcel-bundler-splitable/package.json
@mischnic mischnic merged commit 8d64328 into parcel-bundler:v2 Jul 13, 2020
@devongovett devongovett mentioned this pull request Jul 4, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

V2: JSRuntime should reference child bundle dependencies with a require call when using commonjs
2 participants