search

embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
329 stars 137 forks source link

Adjusting `@embroider/webpack` to use `@babel/preset-env` to avoid critical security audit #1868

Closed lupestro closed 1 month ago

lupestro commented 1 month ago

At its base, this is a straightforward change to one dependency in the package.json for @embroider/webpack.

Running the full test suite locally, the last several release-watch-mode scenarios timed out, but otherwise it showed the same handful of errors that it showed when I ran the test suite before the change. I had seen one or two watch-mode tests fail earlier, so I'm not sure if that's test fragility or something to concern myself with about the change.

I'm also a little concerned about a number of the changes that happened in the pnpm-lock.yaml when I ran pnpm install after making the package.json change. I had only looked at the lock file after I'd run the tests with all those timeouts, though. Not sure if that made a difference.

This PR will at least open the conversation and I can take it from there with guidance and context.

kiwi-josh commented 1 month ago

Chiming in here with some extra thoughts - it appears babel-preset-env is still being included in the pnpm-lock.yaml, because the package.json within the tests include it, and switch on its usage here.

I don't have enough context to understand why/what this is doing

lupestro commented 1 month ago

There is code that deals with both types of babel preset-env based on babel 6 or 7 and the test scenario needs to bring the old one in to test it. It shouldn’t affect the webpack package specifically. Hopefully the audit stuff is making a nuanced enough look at the lock file to recognize that.