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 builtin loaders with --target=node #981

Merged
merged 3 commits into from
Mar 28, 2018
Merged

Conversation

fathyb
Copy link
Contributor

@fathyb fathyb commented Mar 12, 2018

Fixes #969.

Add support for dynamic imports, WebAssembly and Rust with --target=node.

cc @lbguilherme @albizures

  • the global modifications on prelude.js are ugly, any suggestion? Maybe make a Node.js prelude and a browser one?

// require in the next event loop tick to let the main module load
setTimeout(function() {
require(__dirname + bundle)
resolve()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should resolve to the return of require. resolve(require(__dirname + bundle))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure? The original js-loader.js doesn't resolve anything :

script.onload = function () {
script.onerror = script.onload = null;
resolve();
};

If require is called here it's just to register the __parcel_require so next require calls can require numeric IDs here :

return Promise.all(bundles.slice(0, -1).map(loadBundle))
.then(function () {
return require(id);
});

I'm not very familiar with that part though so I may have missed something 😅

@fathyb fathyb added the 📝 WIP Work In Progress label Mar 25, 2018
@devongovett
Copy link
Member

Instead of introducing another global to store the node require function, I just renamed the parcel require function to parcelRequire instead of require so it doesn't conflict with node's require function. This should also fix #761, so that parcel doesn't conflict with the require function exposed by other bundlers or scripts like require.js which might also be on the page. We fall back to checking require if we cannot find the module in parcel's require system.

require.resolve('./builtins/loaders/css-loader')
);
this.addBundleLoader('js', require.resolve('./builtins/loaders/js-loader'));
const loadersPath = `./builtins/loaders/${
Copy link
Member

Choose a reason for hiding this comment

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

would be cool to add support for registering bundle loaders for specific environments. this would allow third party plugins to do the same:

bundler.addBundleLoader('wasm', {
  node: require.resolve('./node/wasm-loader'),
  browser: require.resolve('./browser/wasm-loader')
});

Want to make a followup PR to support that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📝 WIP Work In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants