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

After upgrade to json5@2, rollup bundle fails #182

Closed
plantain-00 opened this issue Aug 18, 2018 · 5 comments
Closed

After upgrade to json5@2, rollup bundle fails #182

plantain-00 opened this issue Aug 18, 2018 · 5 comments
Assignees
Labels

Comments

@plantain-00
Copy link

demo index.js

import JSON5 from 'json5'

console.info(JSON5.stringify('aa'))

demo rollup.config.js

import { uglify } from 'rollup-plugin-uglify'
import resolve from 'rollup-plugin-node-resolve'
import commonjs from 'rollup-plugin-commonjs'

export default {
  input: 'index.js',
  name: 'Test',
  plugins: [
    resolve(),
    uglify(),
    commonjs()
  ],
  output: {
    file: 'index.min.js',
    format: 'umd'
  }
}

rollup --config rollup.config.js

error message

index.js → index.min.js...
  17 |
  18 |     var util = {
> 19 |         isSpaceSeparator (c) {
     |                         ^ Unexpected token: punc (()
  20 |             return unicode.Space_Separator.test(c)
  21 |         },
  22 |
[!] (uglify plugin) Error: Error transforming bundle with 'uglify' plugin: Unexpected token: punc (()
Error: Error transforming bundle with 'uglify' plugin: Unexpected token: punc (()
    at error (/Users/yaoyao/.config/yarn/global/node_modules/rollup/dist/rollup.js:3365:15)
    at /Users/yaoyao/.config/yarn/global/node_modules/rollup/dist/rollup.js:13868:17
    at <anonymous>

It seems uglify-js is able to transpile only es5 syntax, and I'm using json5 in browser environment and target es5.

I'm not sure this is json5's bug(for example if json5 does not target es5 anymore, it is not a bug), feel free to close if not.

@jordanbtucker
Copy link
Member

jordanbtucker commented Aug 18, 2018

json5@2.0.0 no longer targets ES5, and instead it targets Node.js v6 or greater. Moving forward, new major versions of json5 will target the oldest LTS version of Node.js at the time of publishing.

If you're using Node.js v6+, then you probably shouldn't be using rollup-plugin-uglify (or uglify-js) since it doesn't support ES6+. I recommend switching from rollup-plugin-uglify to rollup-plugin-terser, which does support ES6+. If you're using Node.js v5 or lower, then you should stick with json5@1.0.1.

Alternatively, if you want to use json5@2 and rollup-plugin-uglify, you can use the bundle at dist/index.js, which still targets ES5 and works in browser and Node.js environments.

import JSON5 from 'json5/dist/index'

console.info(JSON5.stringify('aa'))

@plantain-00
Copy link
Author

I just tried to import from 'json5/dist/index', it still not target ES5, so not work in browser environment.

@jordanbtucker
Copy link
Member

jordanbtucker commented Aug 18, 2018

Oh, sorry. You're right. The bundle at dist/index.js is just bundled with rollup. The bundle at dist/index.min.js is bundled with rollup-plugin-terser, so it should work be ES5. I'll fix this in 2.0.1.

@jordanbtucker jordanbtucker reopened this Aug 18, 2018
@jordanbtucker
Copy link
Member

Actually, I just realized that rollup-plugin-terser doesn't output ES5 by default either. So json5@2.0.0 has no support for ES5. I'll get that fixed in 2.0.1.

@jordanbtucker
Copy link
Member

Fixed in v2.0.1.

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

No branches or pull requests

2 participants