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

🐛Parcel cannot require a UMD #959

Closed
markandrus opened this issue Mar 7, 2018 · 6 comments
Closed

🐛Parcel cannot require a UMD #959

markandrus opened this issue Mar 7, 2018 · 6 comments

Comments

@markandrus
Copy link

markandrus commented Mar 7, 2018

🎛 Configuration (.babelrc, package.json, cli command)

I setup a project where index.js depends on a UMD. The source of the UMD lives in lib/. It will then be browserified and placed in dist/.

$ tree -I node_modules
.
├── index.js
└── lib
    ├── bar.js
    └── foo.js

1 directory, 3 files

I do this by executing the following:

$ mkdir /tmp/test
$ cd /tmp/test
$ npm install browserify parcel-bundler
$ mkdir lib
$ cat >lib/foo.js <<EOF
> require('./bar');
> EOF
$ touch lib/bar
$ ./node_modules/.bin/browserify lib/foo.js -s Foo -o dist/foo.js
$ cat >index.js <<EOF
> require('./dist/foo');
> EOF

Then, I try to run parcel on index.js.

🤔 Expected Behavior

Parcel is able to bundle the UMD.

$ ./node_modules/.bin/parcel build index.js
⏳  Building...

⏳  Building index.js...
✨  Built in 246ms.

dist/index.js    673 B    20ms

😯 Current Behavior

Parcel fails to bundle the UMD.

$ ./node_modules/.bin/parcel build index.js
⏳  Building...

⏳  Building index.js...
⏳  Building foo.js...
🚨  /private/tmp/test/dist/foo.js:4:8: Cannot resolve dependency './bar' at '/private/tmp/test/dist/bar'
  2 | 
  3 | },{}],2:[function(require,module,exports){
> 4 | require('./bar');
    |         ^
  5 | 
  6 | },{"./bar":1}]},{},[2])(2)
  7 | });

💁 Possible Solution

Webpack succeeds in bundling the UMD, so I think this is doable. I don't know the solution, though.

$ ./node_modules/.bin/webpack index.js -o dist/index.js
Hash: bc32b5444462396b6931
Version: webpack 4.1.1
Time: 260ms
Built at: 2018-3-7 14:39:16
   Asset      Size  Chunks             Chunk Names
index.js  1.05 KiB       0  [emitted]  main
Entrypoint main = index.js
   [0] ./dist/foo.js 959 bytes {0} [built]
   [1] ./index.js 23 bytes {0} [built]

WARNING in configuration
The 'mode' option has not been set. Set 'mode' option to 'development' or 'production' to enable defaults for this environment.

Interestingly, Browserify seems unable to consume its own UMD, which seems like a separate bug.

$ ./node_modules/.bin/browserify index.js -o dist/index.js
Error: Cannot find module './bar' from '/private/tmp/test/dist'
    at /private/tmp/test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:55:21
    at load (/private/tmp/test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:69:43)
    at onex (/private/tmp/test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:92:31)
    at /private/tmp/test/node_modules/browser-resolve/node_modules/resolve/lib/async.js:22:47
    at FSReqWrap.oncomplete (fs.js:166:21)

🔦 Context

I'm trying to replace Webpack with Parcel in a client-side application my team created. We have one or more dependencies that are UMDs.

💻 Code Sample

See above.

🌍 Your Environment

Software Version(s)
Browserify 16.1.1
Webpack 4.1.1
Parcel 1.6.2
Node v9.2.0
npm 5.5.1
Operating System macOS
@davidnagli
Copy link
Contributor

Duplicate of #766

But thanks for bringing it up. We should definitely make sure that UMD support get’s added 👍

@markandrus
Copy link
Author

Hi, @davidnagli — thanks for the response! I'm not sure this is exactly a duplicate, though, since it looks like #766 is about generating UMDs, whereas this is about consuming them. Maybe I could've titled the issue more accurately.

@markandrus markandrus changed the title 🐛Parcel cannot bundle UMD 🐛Parcel cannot require a UMD Mar 8, 2018
@davidnagli
Copy link
Contributor

davidnagli commented Mar 8, 2018

Thanks for the clarification:)

Totally agree that this is something that we should work on 👍

@davidnagli
Copy link
Contributor

@markandrus I’ve been looking into this, and by the looks of it the UMD should be perfectly compatible with CommonJS - and by extension, Parcel.

Can you please provide an example of a UMD module that doesn’t work?

@davidnagli
Copy link
Contributor

For anybody else that is interested in looking into this, and for general reference (since I already did the research anyway), dependencies get collected here:

collectDependencies() {
walk.ancestor(this.ast, collectDependencies, this);
}

walk.ancestor is a method from the babylon-walk package which, as the name implies, walks the Babylon AST (spec). It calls the visitor methods from the collectDependencies visitor object as defined in src/visitors/dependencies.js:

const types = require('babel-types');
const template = require('babel-template');
const urlJoin = require('../utils/urlJoin');
const isURL = require('../utils/is-url');
const matchesPattern = require('./matches-pattern');
const requireTemplate = template('require("_bundle_loader")');
const argTemplate = template('require.resolve(MODULE)');
const serviceWorkerPattern = ['navigator', 'serviceWorker', 'register'];
module.exports = {
ImportDeclaration(node, asset) {
asset.isES6Module = true;
addDependency(asset, node.source);
},
ExportNamedDeclaration(node, asset) {
asset.isES6Module = true;
if (node.source) {
addDependency(asset, node.source);
}
},
ExportAllDeclaration(node, asset) {
asset.isES6Module = true;
addDependency(asset, node.source);
},
ExportDefaultDeclaration(node, asset) {
asset.isES6Module = true;
},
CallExpression(node, asset, ancestors) {
let {callee, arguments: args} = node;
let isRequire =
types.isIdentifier(callee) &&
callee.name === 'require' &&
args.length === 1 &&
types.isStringLiteral(args[0]);
if (isRequire) {
let optional = ancestors.some(a => types.isTryStatement(a)) || undefined;
addDependency(asset, args[0], {optional});
return;
}
let isDynamicImport =
callee.type === 'Import' &&
args.length === 1 &&
types.isStringLiteral(args[0]);
if (isDynamicImport) {
asset.addDependency('_bundle_loader');
addDependency(asset, args[0], {dynamic: true});
node.callee = requireTemplate().expression;
node.arguments[0] = argTemplate({MODULE: args[0]}).expression;
asset.isAstDirty = true;
return;
}
const isRegisterServiceWorker =
types.isStringLiteral(args[0]) &&
matchesPattern(callee, serviceWorkerPattern);
if (isRegisterServiceWorker) {
addURLDependency(asset, args[0]);
return;
}
},
NewExpression(node, asset) {
const {callee, arguments: args} = node;
const isWebWorker =
callee.type === 'Identifier' &&
callee.name === 'Worker' &&
args.length === 1 &&
types.isStringLiteral(args[0]);
if (isWebWorker) {
addURLDependency(asset, args[0]);
return;
}
}
};
function addDependency(asset, node, opts = {}) {
if (asset.options.target !== 'browser') {
const isRelativeImport =
node.value.startsWith('/') ||
node.value.startsWith('./') ||
node.value.startsWith('../');
if (!isRelativeImport) return;
}
opts.loc = node.loc && node.loc.start;
asset.addDependency(node.value, opts);
}
function addURLDependency(asset, node) {
let assetPath = asset.addURLDependency(node.value);
if (!isURL(assetPath)) {
assetPath = urlJoin(asset.options.publicURL, assetPath);
}
node.value = assetPath;
asset.isAstDirty = true;
}

That pretty much sums up how Parcel gathers JavaScript dependencies :)

By the looks of it, this should all be perfectly compatible with UMD, but we’ll have to see what @markandrus answers about the example.

For more info on how visitors work: Here’s a useful explanation from Babel’s plugin handbook

@fathyb
Copy link
Contributor

fathyb commented Mar 9, 2018

@davidnagli you can build the module using OP's instruction, here is ./dist/foo.js :

(function(f){if(typeof exports==="object"&&typeof module!=="undefined"){module.exports=f()}else if(typeof define==="function"&&define.amd){define([],f)}else{var g;if(typeof window!=="undefined"){g=window}else if(typeof global!=="undefined"){g=global}else if(typeof self!=="undefined"){g=self}else{g=this}g.Foo = f()}})(function(){var define,module,exports;return (function(){function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module '"+o+"'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s}return e})()({1:[function(require,module,exports){

},{}],2:[function(require,module,exports){
require('./bar')

},{"./bar":1}]},{},[2])(2)
});

So why does it works with Webpack and not Parcel and Browserify? Because of how we parse the code, Webpack checks if require is already defined the scope while Parcel and Browserify don't. A simplest repro :

index.js

function require(name) {
  console.log('require', name)
}

require('anything')

Boom :

🚨  /private/tmp/test/index.js:5:8: Cannot resolve dependency 'anything'
  3 | }
  4 | 
> 5 | require('anything')
    |         ^
  6 | 
  7 |

Babel offers neat APIs to check this, I'll try to make a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants