Closed henri-hulski closed 7 years ago
It seems, that UglifyJs cannot handle Template strings.
@Guria @christianalfoni Any idea what's going on and why I'm the only person (probably) who gets this errors?
Its true, but how its get there? Travis also builds on 6.9.1 and doesn't catches this error.
Could you show around /usr/lib/nodejs/events.js:260,0
// Check for listener leak
if (!existing.warned) {
m = $getMaxListeners(target);
if (m && m > 0 && existing.length > m) {
existing.warned = true;
const w = new Error('Possible EventEmitter memory leak detected. ' +
`${existing.length} ${type} listeners added. ` +
'Use emitter.setMaxListeners() to increase limit');
w.name = 'Warning';
w.emitter = target;
w.type = type;
w.count = existing.length;
process.emitWarning(w);
}
}
line 260 is
`${existing.length} ${type} listeners added. ` +
When I ran npm run build
with node v5.12.0 before I got this error:
static/js/main.957b2071.js from UglifyJs
SyntaxError: Unexpected token: keyword (const) [/usr/lib/nodejs/util.js:564,0]
But I think now UglifyJs supports const
.
The whole function in /usr/lib/nodejs/events.js is
function _addListener(target, type, listener, prepend) {
var m;
var events;
var existing;
if (typeof listener !== 'function')
throw new TypeError('"listener" argument must be a function');
events = target._events;
if (!events) {
events = target._events = new EventHandlers();
target._eventsCount = 0;
} else {
// To avoid recursion in the case that type === "newListener"! Before
// adding it to the listeners, first emit "newListener".
if (events.newListener) {
target.emit('newListener', type,
listener.listener ? listener.listener : listener);
// Re-assign `events` because a newListener handler could have caused the
// this._events to be assigned to a new object
events = target._events;
}
existing = events[type];
}
if (!existing) {
// Optimize the case of one listener. Don't need the extra array object.
existing = events[type] = listener;
++target._eventsCount;
} else {
if (typeof existing === 'function') {
// Adding the second element, need to change to array.
existing = events[type] = prepend ? [listener, existing] :
[existing, listener];
} else {
// If we've already got an array, just append.
if (prepend) {
existing.unshift(listener);
} else {
existing.push(listener);
}
}
// Check for listener leak
if (!existing.warned) {
m = $getMaxListeners(target);
if (m && m > 0 && existing.length > m) {
existing.warned = true;
const w = new Error('Possible EventEmitter memory leak detected. ' +
`${existing.length} ${type} listeners added. ` +
'Use emitter.setMaxListeners() to increase limit');
w.name = 'Warning';
w.emitter = target;
w.type = type;
w.count = existing.length;
process.emitWarning(w);
}
}
}
return target;
}
Still don't get why not catched on travis. Anyway, either we should use babel for node event emitter, or switch to alternative version from npm.
My vote is second, since we can't control node event emitter dependency. cc @christianalfoni
Though you experiencing this @Guria ? Works fine on my machine
Nope, I haven't managed to get this error yet. But I will investigate it more.
@henri-hulski Could you try npm clean cache
? And do another npm install
?
It is not in npm. It is a node internal module.
Though if UglifyJS is the issue, or whatever builder is using uglifyJs is the issue... might work? ;-)
@christianalfoni It's true, that uglifiJs in my todomvc are not the newest version which is 2.7.4:
pacha@xibalba:~/web/cerebral/cerebral/packages/cerebral-todomvc$ npm list uglify-js
cerebral-todomvc@0.2.0 /home/pacha/web/cerebral/cerebral/packages/cerebral-todomvc
└─┬ react-scripts@0.7.0
├─┬ html-webpack-plugin@2.24.0
│ └─┬ html-minifier@3.1.0
│ └── uglify-js@2.7.3
└─┬ webpack@1.13.2
└── uglify-js@2.6.4
I have made npm cache clean
and deleted node_modules from todomvc and then try to run npm run setup
again, but it seems that I have to slow connection here.
Will try tomorrow.
@christianalfoni Just made a new clone, clean cache, run install, run setup and still the same. The uglify-js versions also stay the same as above.
So no idea, why it happens only to me as this seems to be an install as clean as it can be. ;-)
@henri-hulski Could you force install latest uglifyJS? So it uses 2.7.3 and not 2.6.4?
It seems to be a dependency of html-webpack-plugin 2.24.0 and webpack 1.13.2 inside react-scripts 0.7.0.
I already tried to install newest uglifyJS but it didn't help and react-scripts didn't pick it up. So I can install it inside these packages when I'm back on laptop.
@christianalfoni What do you get for npm list uglify-js
in the packages/cerebral-todomvc directory?
Current uglify-js has no support for es6, as I know. See https://github.com/mishoo/UglifyJS2/issues/448
We can look into different emitters here: https://www.npmjs.com/package/event-emitter https://github.com/jacomyal/emmett https://github.com/primus/eventemitter3 https://github.com/scottcorgan/tiny-emitter https://github.com/facebook/emitter
Take also a look at facebookincubator/create-react-app#984.
They're actually planning to move from uglifyJS to Babili because of this.
Using babili wouldn't transpile template strings into es5 as I can understand. It means that your minified build will contain template strings and it can fail on browsers that still have no support of it.
Babili can be used as a preset for babel. So it should integrate in babel transpile chain.
See a @gaearon comment there. They will not enable transpilation for external modules.
Nevertheless it's important to stress that we aren't and won't be transpiling any dependencies. This is intentional. If your dependencies are published in ES6, your app will not work in browsers that don't support ES6.
[Babili is] ES2015+ aware (nothing special needs to be done because we can use the babylon parser) and Babel will update as standards/browsers update.
See http://126kr.com/article/5by924l0dpq.
@Guria @gaearon also said
ES6 syntax in dependencies is currently not supported because we use Uglify. This will be fixed when we switch to Babili when it's more stable.
Yes, but it will be minified as is without transpilation.
@christianalfoni so maybe switch back to https://github.com/jacomyal/emmett as in Baobab.
@christianalfoni Just to be sure I updated the uglifyJS dependencies to version 2.7.4, but this changed nothing.
@Guria @christianalfoni I replaced the EventEmitter but the error is still the same.
When fixing /usr/lib/nodejs/events.js
, I get similar errors in /usr/lib/nodejs/internal/util.js
:
Failed to compile.
static/js/main.c359f549.js from UglifyJs
Unexpected character '`' [/usr/lib/nodejs/internal/util.js:4,0]
Failed to compile.
static/js/main.3335427e.js from UglifyJs
Unexpected character '`' [/usr/lib/nodejs/internal/util.js:30,0]
Failed to compile.
static/js/main.cebb091d.js from UglifyJs
SyntaxError: Unexpected token: punc (.) [/usr/lib/nodejs/internal/util.js:64,0]
So has nothing to do with our EventEmitter. Seems to be something internal.
No idea!?
We have event emitter in both cerebral and function-tree
Yeah I have replaced both.
@Guria It seems to happen, when the webpack compiler run.
In cerebral/packages/cerebral-todomvc/node_modules/react-scripts/node_modules/webpack/lib/Compiler.js
.
Somewhere in emitRecords:
Compiler.prototype.run = function(callback) {
var startTime = new Date().getTime();
this.applyPluginsAsync("run", this, function(err) {
if(err) return callback(err);
this.readRecords(function(err) {
if(err) return callback(err);
this.compile(function(err, compilation) {
if(err) return callback(err);
if(this.applyPluginsBailResult("should-emit", compilation) === false) {
var stats = compilation.getStats();
stats.startTime = startTime;
stats.endTime = new Date().getTime();
this.applyPlugins("done", stats);
return callback(null, stats);
}
this.emitAssets(compilation, function(err) {
if(err) return callback(err);
this.emitRecords(function(err) {
if(err) return callback(err);
var stats = compilation.getStats();
stats.startTime = startTime;
stats.endTime = new Date().getTime();
this.applyPlugins("done", stats);
return callback(null, stats);
}.bind(this));
}.bind(this));
}.bind(this));
}.bind(this));
}.bind(this));
};
same error here. something else i've ecognised: when running npm install from inside todomvc-folder it starts build script as well?
fops@fopsdev:~/cerebral2/cerebral/packages/cerebral-todomvc$ npm install
> cerebral-todomvc@0.2.0 prepublish /home/fops/cerebral2/cerebral/packages/cerebral-todomvc
> npm run build
> cerebral-todomvc@0.2.0 build /home/fops/cerebral2/cerebral/packages/cerebral-todomvc
> react-scripts build
Creating an optimized production build...
Failed to compile.
static/js/main.738c12f3.js from UglifyJs
Unexpected character '`' [/usr/lib/nodejs/events.js:260,0]
npm install runs prepublish script which calls build script
but where can i see this configuration that npm install starts prepublish? is it by design? thanks for any enlightment
yes, it's a npm built in behaviour
I was debugging this with node-inspector and Chrome devtools. Just running
node --inspect --debug-brk node_modules/react-scripts/scripts/build.js
in the cerebral-todomvc directory.
not sure if it helps, but removing uglify Plugin from webpack.config.prod.js (inside todoMvc/node_modules/react-scripts) solves the issue temporarly for me
Thanks @fops!
It seems to me, that this is actually a bug in react-scripts, as calling node events by webpack during compiling is part of the core functionality.
It is pretty obvious that disabling of Uglify Js "fixes" a problem. But it is no fixes root of problem.
Btw, @henri-hulski, which replacement of eventemitter you have used?
@Guria I tried https://github.com/primus/eventemitter3, which has the same syntax as the node one and is under active development and also https://github.com/jacomyal/emmett, which seems to be a little outdated.
And I think we should file a proper bug report against facebookincubator/create-react-app.
Doesn't looks like CRA bug for me. Just can't get why you got util.js from node in build even after replacing emitter. Btw still can't reproduce it in my environments.
When debugging it the error appears directly after calling emitRecords as shown above. This happens if events.js is fixed or not.
@fopsdev could you tell exactly your environment as I did in the first post, so that we know when the error appears.
@Guria And it seems that when the error appears there isn't yet any Cerebral code involved.
@Guria I just tried to create a minimal app wich throws this error. It's actually enough to add
import {Controller} from 'cerebral';
const controller = Controller();
into index.js to a freshly created CRA to get it.
index.js now looks like:
import React from 'react';
import ReactDOM from 'react-dom';
import App from './App';
import './index.css';
import {Controller} from 'cerebral';
const controller = Controller();
ReactDOM.render(
<App />,
document.getElementById('root')
);
This happens when installing cerebral@2.0.0-alpha.4 and also when linking from the monorepo master.
@Guria So cerebral controller must be somehow involved.
Of course because both cerebral and function-tree based on nodes internal EventEmitter. I just don't get why you still have an issue after moving to alternate emitter. Unfortunately can't recheck it since haven't reproduced it.
Another solution would be to use a create-cerebral-app.
System: Debian GNU/Linux 8.6 (jessie). Linux version: 4.7.0-0.bpo.1-amd64 Node version: 6.9.1 npm version: 3.10.8 react-scripts version: 0.7.0
Just made a fresh clone of the Cerebral repo and ran
npm install
andnpm run setup
which gives me the following error message:When running
npm run build
in thepackages/cerebral-todomvc
directory, I get: