Closed Shyrro closed 1 year ago
Thanks a lot @Shyrro for your contribution! Great results regarding optimization! I understand your point but the problem is that if you don't use
index-with-vars
files, you will have CSS without anyvar()
, so it will not be possible to use light/dark mode.
Hey @lauthieb ! Thanks for your review !
I understand now why the index-with-vars is used instead. I did not know about the Dark mode indeed. Though the fact that we need the vars, pushes the problem to a whole other level :
The vars are imported in the index.ts
file, before, we used to import everything so no issue. With the treeshaking capabilities, we never import the index, it's just an entry point, so the postcss script that registers the vars never gets triggered. This means that to allow for this change, we would need to implement one of these solutions :
From these solutions, the first one would be the easiest to implement but would introduce a Breaking Change. The plugin solution would take some time to implement but is breaking as well since everyone would need to use the plugin.
Considering these issues, feel free to close this PR until something is decided. I know this is not a priority but it would be very nice to consider as not only is it good for Decathlons React apps, it is also good for the environment to not import everything each time. It would also need to be considered for Vue and Svelte whenever we find a viable solution for each.
Hello @Shyrro, thanks again for your contribution. As discussed together, I close this PR but this may be reopen if we accept breaking changes and usage of SemVer in the future for this version of Vitamin. I'll keep you informed. Thanks for your understanding.
Changes description
Support sideEffects to reduce imported size of components.
Currently, the postCss rollup plugin, generates JS that is considered as sideEffects because not a part of our solution and gets treeshaken when
sideEffects: false
is active.Not having the sideEffects specified imports everything, even if you're only importing a button.
Also, we used to use
index-with-vars.css
, which is not necessary since we could need to import and compute the vars at runtime, which is also done by another script which requires a lot of imports too. Using the already computed styles lightens this process and allow for more granular treeshake.As an example, now with a VtmnButton, we only import around 150kb instead of 440kb before. With three components, i am at 170kb.
Context
This improves loading time of all apps and allow people to lazy load some components. Bundle size is lighter and represents more what we use. There is no need to have all the other components in the browser, if we only use a Button and a Tag.
Checklist
design-system-core-team-design
GitHub team.Does this introduce a breaking change?
Other information
Tested on :
Good to know: we can do better, currently, all the side effects are marked, so we always import all the scripts the are generated by the postCSS rollup module, i think we can do better, but we might require better tooling than rollup. So let's do this for now