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

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

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

lupestro
Copy link
Contributor

@lupestro lupestro commented Apr 9, 2024

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.

…preset-env to address critical security audit.
@kiwi-josh
Copy link

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
Copy link
Contributor Author

lupestro commented Apr 9, 2024

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.

@mansona mansona added enhancement New feature or request breaking and removed enhancement New feature or request labels Apr 10, 2024
Copy link
Member

@mansona mansona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ef4 ef4 merged commit 284191d into embroider-build:stable Apr 16, 2024
205 checks passed
@github-actions github-actions bot mentioned this pull request Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants