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

webpack 5 #652

Merged
merged 7 commits into from
Apr 15, 2021
Merged

webpack 5 #652

merged 7 commits into from
Apr 15, 2021

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Dec 8, 2020

Known problems:

Non-blockers with workarounds:

@rwjblue
Copy link
Collaborator

rwjblue commented Mar 10, 2021

The upstream issue (webpack/webpack#12154), we need to investigate if the changes suggested there resolve our issue.

@rwjblue rwjblue marked this pull request as draft March 10, 2021 15:31
@ef4
Copy link
Contributor Author

ef4 commented Mar 10, 2021

Unfortunately their solution doesn't really help us in the backward-compatibility case. We're relying on type javascript/auto because we really do have both import and require.

I still think this is a legitimate bug in their auto mode, but we can just work around it.

ef4 added 5 commits April 14, 2021 15:18
- this gives the app control over the version
 - this lets the app import from webpack so they can access things like `DefinePlugin` without risk of version skew
This was only here to help mho, and it's not really complete enough to work in a build that fully respects "exports"
@ef4 ef4 marked this pull request as ready for review April 14, 2021 22:07
@ef4 ef4 merged commit 92e04c3 into master Apr 15, 2021
@ef4 ef4 deleted the webpack5 branch April 15, 2021 01:33
@ef4 ef4 added the breaking label Apr 15, 2021
@ef4
Copy link
Contributor Author

ef4 commented Apr 15, 2021

This is breaking both because of the webpack 5 upgrade (so apps with custom webpack config may need to adjust it) and because I made webpack a peerDependency of @embroider/webpack, so apps are now required to depend on it directly.

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.

2 participants