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

Upgrade from webpack v4 to v5 #2267

Merged
merged 16 commits into from Jan 30, 2024
Merged

Conversation

GiviMAD
Copy link
Member

@GiviMAD GiviMAD commented Jan 13, 2024

Hello,

I upgraded some packages in order to build the UI with webpack 5 version, I think (hope) I have not break anything.

The reason to do these is because I want to develop a client for these WIP audio protocol openhab/openhab-core#4032, but for it I need to use a WebWorker and a couple of AudioWorkets and webpack 4 have poor/none support for them, so I though it was better trying to update to webpack 5 instead of battle agains the poor v4 support, hope it's ok.

Please let me know if I need to rework anything or the update is unwanted for whatever reason.

Best regards!

PD: the change that worries me more is the upgrade of the YAML library, but the previous version didn't bundle well with webpack 5 (due to using import/export but having not type module in the package json I think).

Copy link

relativeci bot commented Jan 13, 2024

Job #1390: Bundle Size — 11MiB (-30.98%).

0f8db20(current) vs 424e646 main#1388(baseline)

Important

Bundle introduced 3 duplicate packages – View changed duplicate packages

Bundle metrics  Change 10 changes Regression 3 regressions Improvement 3 improvements
                 Current
Job #1390
     Baseline
Job #1388
Regression  Initial JS 1.84MiB(+1.68%) 1.81MiB
Improvement  Initial CSS 607.73KiB(-0.11%) 608.39KiB
Change  Cache Invalidation 95.87% 94.01%
Change  Chunks 220(+1.38%) 217
Change  Assets 242(-64.57%) 683
Change  Modules 3074(-0.13%) 3078
Improvement  Duplicate Modules 159(-32.63%) 236
Regression  Duplicate Code 1.74%(+0.58%) 1.73%
Improvement  Packages 150(-1.32%) 152
Regression  Duplicate Packages 18(+12.5%) 16
Bundle size by type  Change 4 changes Regression 2 regressions Improvement 2 improvements
                 Current
Job #1390
     Baseline
Job #1388
Improvement  JS 9.19MiB (-1.95%) 9.37MiB
Regression  CSS 888.86KiB (+2.92%) 863.61KiB
Not changed  Fonts 526.1KiB 526.1KiB
Not changed  Media 295.6KiB 295.6KiB
Not changed  IMG 140.74KiB 140.74KiB
Regression  HTML 1.24KiB (+1.11%) 1.23KiB
Improvement  Other 871B (-99.98%) 4.78MiB

View job #1390 reportView GiviMAD:upgrade/webpack_5 branch activityView project dashboard

@GiviMAD
Copy link
Member Author

GiviMAD commented Jan 13, 2024

@florian-h05 would you be able to give me your thoughts on these?

I don't know enough about the project to know what to test, I have verified production build and dev server are working correctly and I was going around the UI, and tested a Blockly rule, and everything seems to be working.

Also I'm going to create an issue for the Audio client implementation because I need feedback about how to do it, will ping you there, let me know if I bother you too much. Best regards!

@J-N-K
Copy link
Member

J-N-K commented Jan 14, 2024

Please check what was upgraded in #1353. I believe you need to adjust documentation. IIRC upgrading the tooling for everything (except HABOT) is preferred. If you are done, I can try if we can still build on MacOS.

@GiviMAD
Copy link
Member Author

GiviMAD commented Jan 14, 2024

Please check what was upgraded in #1353. I believe you need to adjust documentation. IIRC upgrading the tooling for everything (except HABOT) is preferred. If you are done, I can try if we can still build on MacOS.

Thank you for the reference, I have updated required versions doc.

I have added a last commit:
The command 'dev:blockly' failed, it works correctly without the use of the "source-map-loader" but, with it, it complains about missing library files in the blocky folder (the code is there but in a single bundle, but when enabling the loader it tries to locate the original files there), seems to work correctly without it.
And also I have disabled the service worker plugin when not in production mode because it produces errors with the hot reload functionality. (I think it can be due to I changed filename: 'js/app.[hash].js' to filename: 'js/app.[contenthash].js', but it was recommended on the v4 to v5 migration docs so I think it's better just disable the service-worker while developing).
And I had removed the explicit use of the HotModuleReplacementPlugin because there was a warning saying it's not needed when the hot: true option is provided to the devServer.

@GiviMAD
Copy link
Member Author

GiviMAD commented Jan 15, 2024

I discovered a problem with the hot reload functionality, I have updated the hotUpdateChunkFilename and hotUpdateMainFilename options based on its documentation for webpack 5.

@GiviMAD
Copy link
Member Author

GiviMAD commented Jan 15, 2024

If you are done, I can try if we can still build on MacOS.

@J-N-K I think I'm done now. I tested with OH_APIBASE=http://...:8080 SOURCE_MAPS=1 npm run dev and I couldn't find any other problems, the hot reload works like charm now.

There is these warning on the build:

(node:62511) [DEP_WEBPACK_COMPILATION_ASSETS] DeprecationWarning: Compilation.assets will be frozen in future, all modifications are deprecated.

Which I think is caused by vue-loader version or other plugin that is not in the latest version, but I think we are good to go like these.
Let me know if you find any problems, I'm working on a macOS but not an Apple Silicon one.

@GiviMAD
Copy link
Member Author

GiviMAD commented Jan 17, 2024

I have updated the babel related dependencies as part of another PR (It gave me some problems with the audio worklets) I will move the code to this one.

@florian-h05
Copy link
Contributor

florian-h05 commented Jan 26, 2024

Hello @GiviMAD,
thanks for your PR.

I will try to have a look soon, but looking at the webpack stats, it seems that something went wrong with the compression of CSS, IMG and HTML, as there is a measurable increase in size: #2267 (comment)

@florian-h05 florian-h05 added enhancement New feature or request main ui Main UI labels Jan 26, 2024
@GiviMAD
Copy link
Member Author

GiviMAD commented Jan 26, 2024

Hello @GiviMAD, thanks for your PR.

I will try to have a look soon, but looking at the webpack stats, it seems that something went wrong with the compression of CSS, IMG and HTML, as there is a measurable increase in size: #2267 (comment)

Thank you for the feedback, after looking to the docs again I think I know what could be the problem (or at least a part of it), the equivalent to the previously used url-loader with a limit seems to be type 'asset' not 'asset/resource', I will try the change to see if the numbers became similar.

@GiviMAD
Copy link
Member Author

GiviMAD commented Jan 26, 2024

Seems like the change has solved the problem for the images and slightly improve it for the css.

GiviMAD and others added 13 commits January 30, 2024 22:03
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

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

Thank you very much for that PR @GiviMAD!

I have now had a very close look, and after fixing some issues it now looks really good - I have even tested the blockly source maps, they still work.

The deprecation warning is gone now, I have just updated a few plugins you did not update.
FYI I discovered an issue where the SSE event stream was not working with the dev server, this was due to compressed: true now being the default, after setting it to false, SSE worked again.

@florian-h05 florian-h05 added this to the 4.2 milestone Jan 30, 2024
@florian-h05 florian-h05 removed the enhancement New feature or request label Jan 30, 2024
@florian-h05 florian-h05 changed the title [UI] Bundle with webpack 5 Upgrade from webpack v4 to v5 Jan 30, 2024
This broke production webpack builds, and it seems that manually adding that plugin is not required:
https://webpack.js.org/plugins/module-concatenation-plugin/,
and we do not have manually set `optimization.concatenateModules`.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor

I have encountered a problem with the production build, which I had to fix.
Now I have tested the dev server with and without blockly source maps and the production build (i.e. deploying the org.openhab.ui bundle to my openHAB server) and everything works fine.

I'll now merge this PR.

@florian-h05 florian-h05 merged commit 1c03c60 into openhab:main Jan 30, 2024
6 checks passed
@GiviMAD
Copy link
Member Author

GiviMAD commented Jan 31, 2024

Thank you so much for fixing the missing things and validate the PR!

florian-h05 added a commit to florian-h05/openhab-webui that referenced this pull request Feb 2, 2024
Regression from openhab#2267.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
florian-h05 added a commit that referenced this pull request Feb 2, 2024
Regression from #2267.

In #2267 the YAML library was upgraded to a new version, but the usage
of the library was not adjusted to breaking changes.
This PR fixes all issues discouvered in the browser console.

---------

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 mentioned this pull request Feb 29, 2024
florian-h05 added a commit to florian-h05/openhab-webui that referenced this pull request Feb 29, 2024
Regression from openhab#2267.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
florian-h05 added a commit that referenced this pull request Feb 29, 2024
Regression from #2267.

This fixes the service-worker and switches from injecting the manifest
into a pre-provided service-worker to generating the service-worker
during webpack builder.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 added the enhancement New feature or request label Mar 2, 2024
@florian-h05 florian-h05 added the dependencies Pull requests that update a dependency file label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants