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 worker bundle hoisting #1257

Merged
merged 6 commits into from
Jul 3, 2018
Merged

Fix worker bundle hoisting #1257

merged 6 commits into from
Jul 3, 2018

Conversation

DeMoorJasper
Copy link
Member

@DeMoorJasper DeMoorJasper commented Apr 27, 2018

This is a branched of PR from #979

Main difference is that this PR adds options to the bundle that sets bundle.isolated, instead of looping through all dependencies every time you add a new one. (Should be slightly faster and more straightforward)
All credit goes to @dhcode for this, just wanted to bring some new life into this issue.

Closes #979

src/Bundler.js Outdated
@@ -532,7 +538,7 @@ class Bundler extends EventEmitter {
bundle = Bundle.createWithAsset(asset);
} else if (dep && dep.dynamic) {
// Create a new bundle for dynamic imports
bundle = bundle.createChildBundle(asset);
bundle = bundle.createChildBundle(asset, dep);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the magical line that will allow me to

import { debounce } from 'lodash';

...in my web workers? We're very keen on being able to bundle external assets in our webworker scripts

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR should fix it

@sebastiencs
Copy link

Thanks for your great work on parcel @DeMoorJasper

I have tested the branch fix/worker-bundle and I still get the error:
Uncaught Error: Cannot find module '99'
when I try to import a module in a worker:

import throttle from "lodash.throttle";

Am I missing something ?

@DeMoorJasper
Copy link
Member Author

DeMoorJasper commented Jun 2, 2018

@sebastiencs that's strange could you create a minimal example repo to reproduce that?

This branch is far behind on the recent parcel version though, so it might actually have been fixed in the master if it's unrelated to this.

@sebastiencs
Copy link

@DeMoorJasper Turns out that if I change my worker filename from worker.tsx to worker.js it works, without changing anything else in the code. I will try to make a minimal repo to reproduce the error.

@devongovett devongovett merged commit 1ab0558 into master Jul 3, 2018
@devongovett devongovett deleted the fix/worker-bundle branch July 3, 2018 08:00
2zH pushed a commit to 2zH/parcel that referenced this pull request Jul 11, 2018
devongovett pushed a commit that referenced this pull request Oct 15, 2018
devongovett pushed a commit that referenced this pull request Oct 15, 2018
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.

5 participants