Simplify constant object access

[adamburgess]

Note: I opened this issue with a higher level question, which was how {foo: 'bar'}.foo === 'bar' should be minified to !0, but this is due to the property accessor not simplifying. If this issue is fixed, then that statement should be minified for free (there is already constant string equality checking). The original issue is at the bottom.

{ foo: 'bar' }.foo should be minified to "bar".
This should work with a fair amount of depth:
{ foo: { bar: 'baz' } }.foo.bar should be minified to "baz".

Would modifying js_parser maybeRewritePropertyAccess be the place for this to go?

Original issue is here. Click to expand.

console.log({foo: 'bar'}.foo === 'bar') is not minified to console.log(!0)
esbuild does minify some other simple statements like this:
console.log('foo' === 'foo') is minified to console.log(!0)

Terser minifies both statements.

---

My story: I was using Snowpack which uses import.meta.env to access the environment, e.g. import.meta.env.NODE_ENV. (instead of process.env.) I added support for this in rollup using the resolveMetaImport plugin hook.

My input:

if (import.meta.env.NODE_ENV === 'development') {
    console.log('dev');
} else {
    console.log('prod');
}

Output from rollup which inlines the import.meta.env access:

if ({"NODE_ENV":"production"}.NODE_ENV === "development") {
  console.log("dev");
} else {
  console.log("prod");
}

with esbuild's minification:

{NODE_ENV:"production"}.NODE_ENV==="development"?console.log("dev"):console.log("prod");

I was hoping that esbuild would recognise this pattern and remove the dead code. Terser's output in this case:

console.log("prod");

---

I understand that esbuild's minifier doesn't aim to support everything, so if that's the case here, all good!

[evanw]

I believe that minifying this should be fine provided that the expression is guaranteed to have no side effects. Some edge cases to think about off the top of my head:

• Anything with computed property names should disable this
• The property must not be a getter, setter, or method
• This must do the right thing in the case of duplicate keys
• This should be disabled if one of the properties is __proto__
• This should probably also never minify to undefined if the key is missing because it may be on the Object prototype

[adamburgess]

If it never minifies to undefined, the existance of __proto__ shouldn't be a problem, right? If the key you're looking for is in the object, take it (as it has precedence), otherwise don't replace the access.

My first version using maybeRewritePropertyAccess is quite simple and seems to work. It only handles property access and string indexer, though (number indexers get skipped: js_parser.go:10600). And direct access only. Tracking something that isn't an anonymous object would require quite a lot more work I bet...

• Anything with computed property names should disable this
• This must do the right thing in the case of duplicate keys

ooh, terser does the wrong thing:

export const computed = {
    a: 'bad',
    [String.fromCharCode(97)]: 'good' // equiv. ['a']: 'good'
}.a; // should be 'good'

export const computed=["bad",String.fromCharCode(97)][0]; // 'bad'

[evanw]

Good catch about terser! I see you have already filed this as terser/terser#980.

If it never minifies to undefined, the existance of __proto__ shouldn't be a problem, right?

I believe you are correct. I was thinking of those rules in order and didn't realize the next rule eclipses that rule.

[evanw]

I just thought of another case:

console.log({
  foo: function() { return this.bar },
  bar: true,
}.foo())

It would be bad if that was changed to this:

console.log(function() { return this.bar }())