-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Update to webpack 5 #26229
Update to webpack 5 #26229
Conversation
I tried this but it gets stuck for me on Just saying so this gets tested with |
@THOUSAND-SKY CI is failing and it is still in draft. Its clearly not ready. |
I know, I wasn't implying that... |
@THOUSAND-SKY I have tested this with matrix-js-sdk and matrix-react-sdk being Note that until element-hq/compound#224 is consumed in matrix-react-sdk, you'll need to manually change this line in With that change done it builds and runs fine for me. |
Alright, I must've messed up somewhere, I wasn't on the same commits as you probably, also I assume I noticed that |
Yes, fair point and testing this is very much appreciated! I've added testing |
This still fails after merging in https://github.com/vector-im/element-web/blob/develop/yarn.lock#L8964 |
compound-web has been updated on In
but the build breaks with
|
Upstream webpack bug: webpack/webpack#14814 It looks like our options are either making the import unanalyzable ( |
And of course Attempting to make the usage unanalyzable in element-hq/compound-web#116. |
This builds and runs now but still fails the type check.
|
I've eliminated worklet-loader but this will only start working once matrix-org/matrix-react-sdk#11845 lands. For the error overlays, I think we could set a flag on the errors as they pass through
When I briefly tried this out, I hit a CSP issue on evaluating the function though. Given that this overlay may easily get frustrating rather than helpful, I opted for just disabling it for all runtime errors for now. I kept it on for build errors because noticing in the browser that you actually haven't successfully rebuilt after a change seemed helpful. |
commit 069c1bc Author: Johannes Marbach <johannesm@element.io> Date: Sat Nov 11 16:08:30 2023 +0100 Replace worker-loader with built-in Webpack 5 support for web workers
Latest status: With matrix-org/matrix-react-sdk#11860 workers and worklets work without worker-loader and worklet-loader. However, worklets require |
I'm, sadly, starting to reach the end of my "wisdom" here in terms of making the audio recording worklet function. Webpack 5 built-in worklet supportThe built-in Webpack 5 support for worklets is what's currently implemented in this PR. When attempting to do a voice recording, it fails with
due to our use of the default Separate entry point for the workletI also tried adding a dedicated entry for the worklet with
and loading the worklet via Judging from the source, this is what https://github.com/reklawnos/worklet-loader does. However, it fails with
When I disable hot reload entirely, this error goes away but it then hits Separate dependent project for the workletI also tried setting up a dedicated project for the worklet.
This, again, requires loading the worklet via Doing this, the worklet runs but only when I 3rd-party audio worklet loaderFinally, I also found and tried https://github.com/leviance/audio-worklet-loader which works but is a fairly young and yet unused project. |
I ended up going the cheap route by creating a tiny custom loader based on https://github.com/reklawnos/worklet-loader specifically for the recorder worklet. This doesn't allow us to use the new Webpack 5 syntax but it does work and it does cause any peer-dependency warnings. |
Requires: matrix-org/matrix-react-sdk#11869
I have deliberately left most of the optional changes and deprecation fixes out of this pull request to keep the diff small. Those should be easy to follow up on afterwards.
Things tested
yarn build
yarn start
Open questions
--output-filename
and--output-chunk-filename
flags that stopped working in Update webpack to 4.47.0 / webpack-cli to 4.10.0 / webpack-dev-server 4.15.1 #26216 have been restored in this pull request. However, I'm unsure what effect these options have. I would have expected to see a_dev_
directory but didn't.index.ts
, couldsetimmediate
be injected via aProvidePlugin
, too?Granular tasks
Webpack 5 migration guide: https://webpack.js.org/migrate/5/
--output-filename
and--output-chunk-filename
flags that stopped working in Update webpack to 4.47.0 / webpack-cli to 4.10.0 / webpack-dev-server 4.15.1 #26216namedModules: false
tomoduleIds: "named"
node
toresolve.fallback
VectorAuthPage
It tries to look for
./VectorAuthFooter
in matrix-react-sdk instead of element-web. SinceVectorAuthFooter
is substituted forAuthFooter
in the build, I simply imported that from matrix-react-sdk and inserted it. This appears to work when running the app.There indeed is. I've been able to work around it by setting
context
to the replaced-with file's parent directory. See NormalModuleReplacementPlugin doesn't resolve relative imports of also replaced modules within replaced modules anymore in webpack 5 webpack/webpack#17716 for upstream tracking.index.html
to ignore head tags withouthref
HtmlWebpackPlugin now includes further tags (the bundles), too. It's possible to prevent this with the
scriptLoading: "blocking"
option but being more graceful seems sensible regardless.ProvidePlugin
)ProvidePlugin
)index.ts
because I couldn't figure out how to do it withProvidePlugin
)useId
didn't break with webpack 4 (useId not available in React 17 compound#224, Make usage of useId unanalyzable compound-web#116)The
.
prefix for fonts inindex.html
is not required anymore since the path is already relative in Webpack 5yarn link
ing dependencies still worksFixes: https://github.com/vector-im/wat-internal/issues/67
This change is marked as an internal change (Task), so will not be included in the changelog.