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

import json does not use object shorthand with es2015 option #4257

Open
dnalborczyk opened this issue Oct 26, 2021 · 4 comments
Open

import json does not use object shorthand with es2015 option #4257

dnalborczyk opened this issue Oct 26, 2021 · 4 comments

Comments

@dnalborczyk
Copy link
Contributor

dnalborczyk commented Oct 26, 2021

not sure if that is a bug, feature request, or modification request. I'm also not sure if that's the json plugin's responsibility. I'll have to look into it.

Rollup Version

2.58.3

Operating System (or Browser)

n/a

Node Version (if applicable)

n/a

Link To Reproduction

n/a

Expected Behaviour

config.json

{
  "foo": "bar"
}

index.js

import config from "./config.json"

console.log(config);

rollup.config.js

import json from "@rollup/plugin-json";

export default {
  input: "./src/index.js",
  output: {
    file: "./build/bundle.min.js",
    format: "module",
    generatedCode: "es2015", // <---- allow object shorthand (and others)
    name: "bundle",
  },
  plugins: [json()],
};

generates:

var foo = "bar";
const config = {
	foo
};

console.log(config);

I might even expect this, but I'm sure there's a reason:

const config = {
	foo: "bar"
};

console.log(config);

Actual Behaviour

var foo = "bar";
const config = {
	foo: foo // <--- no object shorthand
};

console.log(config);
@kzc
Copy link
Contributor

kzc commented Oct 26, 2021

There's no real advantage, except for slightly more readable non-minified output. Once minified it doesn't matter much.

$ terser actual.js --toplevel --mangle --compress
const o={foo:"bar"};console.log(o);

$ terser actual.js --toplevel --mangle --compress passes=2
console.log({foo:"bar"});

@lukastaegert
Copy link
Member

In any case, this would be a feature request for the json plugin to respect the flag. Rollup core does not import json files.

@dnalborczyk
Copy link
Contributor Author

There's no real advantage, except for slightly more readable non-minified output. Once minified it doesn't matter much.

I agree, although it would be less code. I was under the impression that's (one of the) reasons the generatedCode option was introduced, but I suppose it's only for import/export syntax related things?

In any case, this would be a feature request for the json plugin to respect the flag. Rollup core does not import json files.

was only looking into current json rollup behavior regarding json import assertions/json modules and I'm thinking that it should (eventually/v3) go into core for json import assertions. 🤔

@kzc
Copy link
Contributor

kzc commented Oct 26, 2021

I think it's more of a feature request rather than a bug - the plugin generated code does work correctly.

afaik @rollup/plugin-json outputs the JSON data in pieces so that Rollup can simplify it, but popular minifiers can often simplify the original form:

$ echo 'var o = {a:1, b:2}; console.log(o.a);' | rollup --silent
var o = {a:1, b:2}; console.log(o.a);
$ echo 'var o = {a:1, b:2}; console.log(o.a);' | terser --toplevel -mc
console.log(1);

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

No branches or pull requests

3 participants