rollup/rollup-plugin-json

Quoted properties generate invalid JavaScript

tmcw opened this issue · 6 comments

tmcw commented

I'm using rollup with the iD project and it's lovely! We're running into one problem importing a large JSON file that includes escaped quotes as property keys. rollup-plugin-json transforms these keys, unescaping the quotes and producing invalid JavaScript:

2016-08-09 at 1 23 pm

I'll push a failing testcase in a moment.

tmcw commented

Would it be better to

  • Error out on this value and provide a module that exposes a JSON file only in its full form rather than exposing a tree-shaking-compatible interface?
  • Or doing what Elm does and automatically making the properties accessible with camelCase names?

@tmcw The first options sounds best to me. Transforming property names will only bring about more headache. How will we transform something like this in a way that is intuitive?

{
  "fooBar": 1,
  "foo bar": 2,
  "foo-bar": 3
}

I'm running into this in a build. It seems a package.json in my node_modules contains quotes inside key names. However, I don't quite understand the issue, or rather the suggested resolution.

Why are we talking about either transforming keys or not converting the JSON to objects as solutions?

To my knowledge JavaScript will happily accept properties with any name (including with quotes), they just can't be accessed with dot syntax, which is unimportant for tree shaking.

Isn't it just a matter of ensuring we generate valid JavaScript (i.e. using bracket syntax when keys aren't valid "identifiers")?

tmcw commented

Yes, you're misunderstanding. Modules in CommonJS are declared as properties of an object:

module.exports['foo bar'] = 2;

But ES6 module syntax, module exports are identifiers, not properties:

export var 'foo bar' = 2; // invalid
export var foo = 2; // valid

And named imports import named identifiers:

import { 'foo bar' } from './module'; // invalid
import { foo } from './module'; // valid

Ah, right, because ES6 lacks export foo as *;

A bit obscure, but better than erroring, I imagine you can grab all the exports that aren't valid identifiers and go:

var badExports = {};
badExports['not a valid identifier'] = foo;
export { badExports as __badExports };

Then they can still be accessed, albeit awkwardly with:

import { __badExports as bar } from 'wherever';
tmcw commented

So! This was fixed in 0fe4fc1