Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Conflict detection/avoidance for "require" aliases #232

Closed
otsdr opened this issue Sep 26, 2017 · 8 comments
Closed

Conflict detection/avoidance for "require" aliases #232

otsdr opened this issue Sep 26, 2017 · 8 comments

Comments

@otsdr
Copy link

otsdr commented Sep 26, 2017

React 16 bundles now use rollup as well; this means some of them may contain aliased requires, which will clash with the ones generated by the local rollup.
Example:
React 16.0.0.0, node_modules/react/cjs/react.development.js

var objectAssign$1 = require('object-assign');
var require$$0 = require('fbjs/lib/warning');

gets converted to

var objectAssign$1 = require$$0;
var require$$0 = warning_1;

, which is obviously invalid.

This situation could be avoided if the name candidates were tested for uniqueness while increasing the uid.
Alternatively, either the separator ($$) or the start uid (0) could be configurable in transform.js .

@otsdr
Copy link
Author

otsdr commented Sep 26, 2017

I have attached a small project that exhibits this issue. The production build does not go through the code block that contains the conflicting variable.
Ultimately, the resulting dev bundle crashes with:
Uncaught TypeError: objectAssign$1 is not a function

$ cat /etc/issue
Ubuntu 17.04
$ yarn --version
1.1.0
$ nodejs --version
v6.11.3
$ npm --version
3.10.10
$ yarn install
(...)
$ npm run-script build

[email protected] build
rollup -c
./src/index.tsx → ./dist/bundle.js...
created ./dist/bundle.js in 3.3s

repro.zip

@mbostock
Copy link
Contributor

Tiny test case here: rollup/rollup#1655 (comment)

mbostock added a commit to mbostock/rollup-plugin-commonjs that referenced this issue Sep 26, 2017
zwhitchcox referenced this issue in yamafaktory/babel-react-rollup-starter Oct 5, 2017
fix(package): update react-dom to version 16.0.0
mbostock added a commit to mbostock/rollup-plugin-commonjs that referenced this issue Oct 7, 2017
lukastaegert added a commit that referenced this issue Oct 7, 2017
@lukastaegert
Copy link
Member

Hi @mbostock et al, as it appears that @Rich-Harris is currently on an extended vacation with limited internet access and cannot release a new version to npm, I have set up a make-shift solution for anyone suffering from this bug. You can now

npm install github:rollup-plugin-commonjs@rollup/rollup-plugin-commonjs#bundled-v8.2.3

which should contain all necessary build artefacts.

farmasek added a commit to WeaConPi/WeaRea that referenced this issue Oct 8, 2017
@hansoksendahl
Copy link

Any ETA on an npm release? It took a lot of googling to get here.

@lukastaegert
Copy link
Member

Unfortunately not. It seems @Rich-Harris is on extended vacation with limited internet access and I do not know who else can publish to npm. Until then I fear you need to stick with the github install.

@Itee
Copy link

Itee commented Oct 12, 2017

Work perfectly for me ! Thank for the set-up.

I hope this is not vacation around the world ?!

rtsao pushed a commit to rtsao/create-universal-package that referenced this issue Oct 18, 2017
@mijamo
Copy link

mijamo commented Oct 20, 2017

It seems like 8.2.4 was published yesterday on NPM so it is now fine :)

@falconmick
Copy link

I'm getting

TypeError: Object(...) is not a function

now, is this related?

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

No branches or pull requests

7 participants