|
Description
Paul Derscheid
2024-09-03 15:32:46 UTC
Created attachment 170992 [details] [review] Bug 37824: Replace webpack with rspack for fun and profit I left the webpack.config.js as well as the devDependencies in place for now. We can remove them in a follow-up patch on this bug after testing it out. To test: 1) Run js:build and js:build:prod 2) Note the build time 3) Apply patch 4) Run yarn install 5) Run js:build and js:build:prod again 6) Note the much faster build time 7) Extra credit: take a look at the ERM or preservations module and make sure everything works as expected. 8) Extra credit: run the cypress tests. 9) Sign off or give your opinion Created attachment 171126 [details] [review] Bug 37824: Replace webpack with rspack for fun and profit I left the webpack.config.js as well as the devDependencies in place for now. We can remove them in a follow-up patch on this bug after testing it out. To test: 1) Run js:build and js:build:prod 2) Note the build time 3) Apply patch 4) Run yarn install 5) Run js:build and js:build:prod again 6) Note the much faster build time 7) Extra credit: take a look at the ERM or preservations module and make sure everything works as expected. 8) Extra credit: run the cypress tests. 9) Sign off or give your opinion Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Looks faster to me. Where webpack failed for me (Killed, error 137), this passed. Concerning the production readiness of rspack, I'd just like to say that ByteDance themselves already use it in production. And I guess what works for a billion $ company should be stable enough for us? I don't have the time to test it right now but if it's faster and considered stable we should switch! There's a solid argument joubu made against mine: ByteDance as well as the big testemonials: - Microsoft - Amazon - Intuit - Discord have time and resources to adapt their codebases to changes in the build tool and we don't. However: they aim to reach as close to 100% compatibility as possible with webpack, which to me means that: - they can't do crazy stuff - we can always fallback onto webpack, it's not going anywhere. Therefore I think we can be a bit bleeding edge here ;) I'll be taking this for a spin today. Now that I've researched the current state of these sorts of tools I'm happy for us to take the leap too. There's a clear back step should we need to take it, but the project itself does feel well regarded and supported so I don't predict we'll need to take such measures. Assuming I get the same results as prior testers, and I can't see why I wouldn't, 8 think this is well worthwhile and likely to pass today. Thanks for testing this Martin :) Created attachment 171438 [details] [review] Bug 37824: Replace webpack with rspack for fun and profit I left the webpack.config.js as well as the devDependencies in place for now. We can remove them in a follow-up patch on this bug after testing it out. To test: 1) Run js:build and js:build:prod 2) Note the build time 3) Apply patch 4) Run yarn install 5) Run js:build and js:build:prod again 6) Note the much faster build time 7) Extra credit: take a look at the ERM or preservations module and make sure everything works as expected. 8) Extra credit: run the cypress tests. 9) Sign off or give your opinion Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Looks faster to me. Where webpack failed for me (Killed, error 137), this passed. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> I'm happy this is a solid improvement and worthwhile. All worked in testing very seamlessly. Passing QA Nice work Paul Just wondering: do we need to do anything to make sure the yarn build step is done on other ktd, Jenkins, sandboxes? Only of they directly call webpack which I hope is not the case. If everything is just yarn *:* and the node_modules is up-to-date, it will just work (and better than before imo). Pushed for 24.11! Well done everyone, thank you! Failure at ktd startup koha-1 | $ rspack build --mode development koha-1 | [rspack-cli] [Error: × Rspack FS Error: Error: EACCES: permission denied, open '/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/js/vue/dist/erm.js.map' koha-1 | │ koha-1 | ╰─▶ Error: EACCES: permission denied, open '/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/js/vue/dist/erm.js.map' (In reply to Jonathan Druart from comment #13) > Failure at ktd startup > > koha-1 | $ rspack build --mode development > koha-1 | [rspack-cli] [Error: × Rspack FS Error: Error: EACCES: > permission denied, open > '/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/js/vue/dist/erm.js.map' > koha-1 | │ > koha-1 | ╰─▶ Error: EACCES: permission denied, open > '/kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/js/vue/dist/erm.js.map' I chowned my whole repo and now ktd is starting correctly. Cypress tests failing since this change:
% yarn cypress run --spec t/cypress/integration/ERM/Agreements_spec.ts
1) Agreement CRUD operations
Delete agreement:
Error: The following error originated from your application code, not from Cypress. It was caused by an unhandled promise rejection.
> Cannot extend unknown button type: excelHtml5
When Cypress detects uncaught errors originating from your application it will automatically fail the current test.
This behavior is configurable, and you can choose to turn this off by listening to the `uncaught:exception` event.
https://on.cypress.io/uncaught-exception-from-application
Taking a look! We have another failing test here: t/Makefile.t I think we need to put the new config file in this list: https://git.koha-community.org/Koha-community/Koha/src/branch/main/Makefile.PL#L298 Created attachment 171552 [details] [review] Bug 37824: (follow-up) Add rspack.config.js to Makefile.PL Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Follow-up for Makefile.PL pushed to main. Created attachment 171568 [details] [review] Bug 37824: (QA follow-up) Fix improper handling of jQuery and DataTables libraries by marking them as externals This commit addresses an issue where jQuery and DataTables libraries (datatables.net and related extensions) were not functioning correctly when bundled with rspack. These libraries expect to be loaded in the global scope rather than as bundled modules, leading to initialization issues. To resolve this, jQuery and all related DataTables libraries (datatables.net, datatables.net-buttons, datatables.net-buttons/js/buttons.html5, etc.) are marked as externals in the rspack configuration. This ensures they are treated as global dependencies, preventing conflicts and allowing proper initialization. Created attachment 171909 [details] [review] Bug 37824: remove references to non-existent files in Makefile.PL to test... 0/ install debhelper tool 1/ run 'DEB_BUILD_OPTIONS=nocheck dh build', observe failure make: *** No rule to make target 'koha-tmpl/intranet-tmpl/prog/js/vue/dist/erm.js.LICENSE.txt', needed by 'pm_to_blib'. Stop. dh_auto_build: error: make -j1 returned exit code 2 2/ apply patch 3/ run 'DEB_BUILD_OPTIONS=nocheck dh build', observe success Created attachment 172113 [details] [review] Bug 37824: remove references to non-existent files in Makefile.PL to test... 0/ install debhelper tool 1/ run 'DEB_BUILD_OPTIONS=nocheck dh build', observe failure make: *** No rule to make target 'koha-tmpl/intranet-tmpl/prog/js/vue/dist/erm.js.LICENSE.txt', needed by 'pm_to_blib'. Stop. dh_auto_build: error: make -j1 returned exit code 2 2/ apply patch 3/ run 'DEB_BUILD_OPTIONS=nocheck dh build', observe success Signed-off-by: Blou <blou@inlibro.com> Tested Mason's patch on the main, and it resolved the issue. This otherwise prevents all usage of main (including signoffs, etc..) (In reply to Blou from comment #23) > Tested Mason's patch on the main, and it resolved the issue. > > This otherwise prevents all usage of main (including signoffs, etc..) Pushed the follow-up to main. |