Invalid code output when using `@embroider/macros` (ember app)

[nulle]

I have an issue of @embroider/macros installation messing up the output of my Ember app code. From what I'm guessing, it's got something to do with (incorrect) optional chaining operator implementation, and appears only in production builds.

Reproduce steps:

1. Create a fresh EmberJS app
2. Add the following code somewhere

const test = {
    items: [],
};
[1, 2].forEach((i) => {
    test.items.push(i);
});
return String(test.items?.[0]);

3. ember serve --environment=production

4. The following code outputs "1". <-- this is the expected output (first array item)

5. install embroider/macros package npm install @embroider/macros --save-dev

6. ember serve --environment=production

7. The same code outputs "undefined" <-- not good

Not only the result is undefined, but if you look at the assets file, it's actually compiled as such: String(void 0)

There is also a minimum viable reproduce here: https://github.com/nulle/testapp/pull/1

Note: the only reason I'm adding @embroider/macros is for ember-data randomUUID polyfill.

[NullVoxPopuli]

I was looking at the compiled code for your snippet, and it looks like the stuff inside String is converted to void

class r extends t.default {
  get test() {
    const e = { items: [] };
    return (
      [1, 2].forEach((t) => {
        e.items.push(t);
      }),
      String(void 0)
    );
  }
}

If I change the input code to:

    return `${test.items?.[0]}`;

The output is now:

class r extends t.default {
  get test() {
    const e = { items: [] };
    return (
      [1, 2].forEach((t) => {
        e.items.push(t);
      }),
      'undefined'
    );
  }
}

I had hypothesized that this could be related to the test word used in the input code, but after changing the input code to

import Controller from '@ember/controller';

export default class ApplicationController extends Controller {
  get foo() {
    const bar = {
      items: [],
    };
    [1, 2].forEach((i) => {
      bar.items.push(i);
    });
    return String(bar.items?.[0]);
  }
}

the production output is the same as the first (void)

next hypothesis is that this is actually a terser issue (maybe it's optimizing something that I can't see?) -- so with the reproduction branch, I've uninstalled @embroider/macros... but what we expect to exist in the production output is there:

class r extends t.default {
  get foo() {
    const e = { items: [] };
    return (
      [1, 2].forEach((t) => {
        e.items.push(t);
      }),
      String(e.items?.[0])
    );
  }
}

so that basically confirms this is an issue with the babel plugins that @embroider/macros is bringing in. (and this is re-confirmed after I added @embroider/macros back to this project).

If we're thinking that it's macros' babel plugin causing this behavior, a place to debug would be here: https://github.com/embroider-build/embroider/blob/main/packages/macros/src/ember-addon-main.ts#L76

and here https://github.com/embroider-build/embroider/blob/main/packages/macros/src/babel/macros-babel-plugin.ts#L15

And if you wanted to submit a failing test PR, you could add your scenario here: https://github.com/embroider-build/embroider/tree/main/packages/macros/tests/babel

[CvX]

We hit this bug in https://github.com/discourse/discourse too. The repro can be reduced to:

let x = [];
x?.push();

(or even code-golfed to []?.at() 😉)