[VO-1282] chore: Migrate to Rsbuild #3296
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BundleMonFiles added (19)
Files removed (16)
Files updated (1)
Unchanged files (1)
Total files change -596.49KB -10.09% Groups added (3)
Groups removed (7)
Final result: ✅ View report in BundleMon website ➡️ |
Ldoppea
force-pushed
the
feat/migrate-to-rsbuild
branch
2 times, most recently
from
98c82cd to
7201534
Compare
|
This commit title seems incomplete |
zatteo
reviewed
Jan 15, 2025
zatteo
reviewed
Jan 15, 2025
|
Great job! 🎉 Can't wait to build faster than light ✨ |
zatteo
force-pushed
the
feat/migrate-to-rsbuild
branch
2 times, most recently
from
cb626f2 to
2f7e44e
Compare
Ldoppea
force-pushed
the
feat/migrate-to-rsbuild
branch
4 times, most recently
from
6975fba to
67e92a4
Compare
In #3182 we reworked Breadcrumbs to display the root folder and Shared drives' root as an icon instead of text Unit tests did not reflect this changes but did continue to pass due to a bug in cozy-client-js exports making the ROOT_DIR_ID undefined Since we removed cozy-client-js and used a defined constant instead then unit tests would fail This commit fixes all the tests to reflect the new "icon" behavior
With the Rsbuild migration, the transpiled code now use `let` and `const` features contrary to the previous Webpack configuration that converted them to `var` This highlight some bug that we previously missed where some variables were used before being declared. This did not produce any throw when using `var` due to hoisting, but this may produce bug as the accessed variable may be undefined
With the Rsbuild migration, the transpiled code now use `let` and `const` features contrary to the previous Webpack configuration that converted them to `var` This highlight some bug that we previously missed where some variables were used before being declared. This did not produce any throw when using `var` due to hoisting, but this may produce bug as the accessed variable may be undefined Warning: This would fix the `fil-content-row-selected` class attribution, however this class is not compatible yet with dark theme, so we want to enforce `undefined` until we fix the theme. Until then the behaviour will be like before when the `selected` variable was always `undefined`
In cozy-script we used to enforce the development mode when doing a
`yarn watch`
The development mode changes the build behavior by removing some steps
like minification which results to a faster build
A side effect is that the code generated from the `watch` command is a
bit different than the one generated from `build` command. This may
produce some differences on the app's behavior, but this is rare enough
(happened 1 or 2 times in the past years) so we consider the speed gain
to still be valuable. Also we know that we should run a `build` locally
as ultimate check before pushing new code to the git repo
So we want to enable this mode in the new Rsbuild configuration
Here are the timings for initial build and then 4 differents edits in
the code that trigger a re-build
yarn watch
```
11,70s (initial build)
9,87s
9,89s
10,10s
10,80s
```
yarn watch --mode development
```
6,64s (initial build)
1,42s
0,73s
0,72s
0,77s
```
Related code:
https://github.com/cozy/create-cozy-app/blob/master/packages/cozy-scripts/scripts/watch.js#L13
`rsbuild-config-cozy-app` has been upgraded to `0.2.0` in order to retrieve the configuration needed to run `rsbuild dev` with our cozy-stack based architecture and then benefits from HMR feature Related PR: cozy/cozy-libs#2700
Since previous commit we can run `rsbuild dev` to enable HMR feature We now have 3 different scripts: - `yarn build`: build the app for production - `yarn watch`: like for the `build` script but with some optimizations in order to build faster and to trigger rebuild automatically when the code changes - `yarn dev`: build the app with HMR enabled. This should be the fastest way to debug the app, but the HMR feature may prevent the app to work in some environments (i.e. flagship app)
`cozy-dataproxy-lib` has been upgraded to `2.3.1` in order to retrieve a fix on package.json configuration that did not export the dataproxy's `stylesheet.css` Related PR: cozy/cozy-libs#2699
By migrating from webpack to rsbuild we lost this resolution override that is needed by react-pdf for generating PDFs previews using WebWorkers More details: https://github.com/cozy/cozy-libs/blob/a00baa1723bd7d37591fe95dcc1921d4e2c24f87/packages/cozy-viewer/src/Readme.md?plain=1#L299-L335
In previous commit we removed cozy-client-js so we now want Intents to work with cozy-client instead
This intent seems to be unused anymore as no call can be found in our entire code base
Since the react-pdf's webpack resolver is declared in `rsbuild.config.mjs` then we don't need to manually enforce a worker in PublicViewer A worker will be used anyway thanks to the webpack resolver
`rsbuild-config-cozy-app` has been upgraded to `0.2.1` in order to retrieve the configuration needed to run intents and public targets in the `rsbuild dev` mode Related PR: cozy/cozy-libs#2717
Ldoppea
force-pushed
the
feat/migrate-to-rsbuild
branch
from
67e92a4 to
5aeb3c3
Compare
zatteo
approved these changes
Jan 30, 2025
JF-Cozy
reviewed
Jan 30, 2025
package.json
Outdated
| "watch": "cozy-scripts watch --barV7 false --cozyClientJs", | ||
| "start": "cozy-scripts start --barV7 false --cozyClientJs", | ||
| "build": "rsbuild build", | ||
| "watch": "rsbuild build --watch", |
There was a problem hiding this comment.
on pourrait garder le start ici non ?
There was a problem hiding this comment.
pourquoi avoir déplacé les assets statiques dans /public ? Y'a t-il une raison en lien avec rsbuild ?
| @@ -24,19 +24,11 @@ | |||
| name="viewport" | |||
| content="width=device-width, height=device-height, initial-scale=1, viewport-fit=cover" | |||
| /> | |||
| <% if (__STACK_ASSETS__) { %> | |||
There was a problem hiding this comment.
le __STACK_ASSETS__ c'est pas les overrides ? Est-ce qu'on ne perd pas cette feature au passage ?
There was a problem hiding this comment.
à quoi ça sert si les alias "directs" (genre modules, components etc.) existent 🤔 Ne faudrait-il pas les supprimer au passage ? Garder les deux approches me semble inutile non ?
There was a problem hiding this comment.
on avait pas déplacé les assets dans /public justement ? 🤔
| const filContentRowSelected = cx(styles['fil-content-row'], { | ||
| [styles['fil-content-row-selected']]: selected, | ||
| [styles['fil-content-row-selected']]: undefined, // TODO: replace with `selected` and fix dark theme for this variable |
There was a problem hiding this comment.
est-ce que le TODO est traité plus tard ?
| @@ -9,7 +9,6 @@ | |||
| "cozyPublish": "cozy-app-publish --token $REGISTRY_TOKEN --prepublish downcloud --postpublish mattermost", | |||
| "tx": "tx pull --all || true", | |||
| "lint": "npm-run-all --parallel 'lint:*'", | |||
| "lint:styles": "stylint src --config ./node_modules/cozy-scripts/config/.stylintrc", | |||
There was a problem hiding this comment.
est-ce qu'on le remet plus tard ?
| @@ -5,6 +5,7 @@ | |||
| "scripts": { | |||
| "build": "rsbuild build", | |||
| "watch": "rsbuild build --watch --mode development", | |||
| "start": "rsbuild dev", | |||
There was a problem hiding this comment.
nit: le commentaire du commit n'est pas raccord avec le code (yarn dev d'un côté, yarn start de l'autre)
| @@ -92,6 +92,7 @@ | |||
| "cozy-flags": "^4.6.1", | |||
| "cozy-harvest-lib": "^32.2.6", | |||
| "cozy-intent": "^2.29.1", | |||
| "cozy-interapp": "^0.15.1", | |||
There was a problem hiding this comment.
pourquoi donc ? C'était une coquille sur Drive ou c'est en lien avec la migration ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR migrate the project's bundler from Webpack to Rsbuild
Non-regression tests have been done:
OPENintentyarn service build/services/<serviceFileName>Warning
I identified a bug in the
File.jsxcode where we apply thefil-content-row-selectedclass on selected filesPreviously the variable was always
undefined. If I fix this line, I introduce a visual bug on Dark theme, so I chose enforce theundefineduntil we fix the theme on another PRSee the
// TODOcomment in theFile.jsxfilePerformances comparison on my computer:
yarn buildyarn watchRelated PRs: