calvinmetcalf / rollup-plugin-node-builtins

138 stars 40 forks source link

EventEmitter - Template literal not being transpiled #16

Open notdotscott opened 7 years ago

notdotscott commented 7 years ago

Hi, I came across an error with the latest changes to EventEmitter. A template literal was added here: https://github.com/calvinmetcalf/rollup-plugin-node-builtins/blob/master/src/es6/events.js#L239 but it doesn't seem to get transpiled when using builtIns() as part of a rollup bundle.

My main.js is a one liner importing EventEmitter:

import EventEmitter from 'events';

My rollup.config.js looks like this:

import builtins from 'rollup-plugin-node-builtins';

export default {
    entry: 'main.js',
    format: 'cjs',
    plugins: [ builtins() ],
    dest: 'bundle.js'
};

After running rollup the bundle.js file still contains the template literal (line 236):

`${existing.length} ${type} listeners added. ` +

This is causing issues in some production code that I'm trying to uglify using rollup-plugin-uglify - uglifyJS throws an error when encountering the back tick:

SyntaxError: Unexpected character \'`\''
DPr00f commented 7 years ago

Good catch. Can you open a PR?

notdotscott commented 7 years ago

Oh, and a prerequisite to running the browser tests is having serve available via the command line, which I didn't.

Fixed by: npm install --save-dev serve

And changing the browser-test package script to: node_modules/serve/bin/serve browser-test/dist

(I didn't want to install serve globally).

calvinmetcalf commented 7 years ago

you only need to do the dev install of serve, you don't need to change the browser-test package, npm scripts automatically put node_modules/.bin into the path

calvinmetcalf commented 7 years ago

should be fixed in #16

notdotscott commented 7 years ago

Hey, just noticed the same issue with util. There's an import statement at the top that the build complains about. Am I right in thinking that the rollup plugins can't be written in es6 as they don't get transpiled? If so you'll have to remove all es6 code from the node builtins :(

calvinmetcalf commented 7 years ago

This is a different issue where the commonjs plugin sees global being used and confuses it for a commonjs module, try passing the noGlobals option (may be spelled diff) to that plugin

On Tue, Oct 18, 2016, 6:02 PM d-scott notifications@github.com wrote:

Hey, just noticed the same issue with util. There's an import statement at the top that the build complains about. Am I right in thinking that the rollup plugins can't be written in es6 as they don't get transpiled? If so you'll have to remove all es6 code from the node builtins :(

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/calvinmetcalf/rollup-plugin-node-builtins/issues/16#issuecomment-254535931, or mute the thread https://github.com/notifications/unsubscribe-auth/ABE4nw4aMLET3f2MtE1za5l_rE2pOutNks5q1N-QgaJpZM4KJ1AD .

notdotscott commented 7 years ago

You mean the ignoreGlobal flag? I'm not specifying it and it defaults to false anyway, so I don't think it was that. However, I did update all my babel and rollup dependencies and it all works now, so I can't actually tell you exactly what the issue was unfortunately! Thanks for the help :) You want me to close this issue now?