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

refactor: replace webpack with vite #1944

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Conversation

ManiacMaxo
Copy link

Context

Vite + SWC is faster and more modern than Webpack + Babel.

Description

Migrated the dev + build process from webpack to vite. All webpack plugins seem to be supported in one way or another in vite so the output should be basically the same.

Some things to note:

  • Ouput index.js is up from 1MB to 2MB. Mainly due to the graphql operations + apollo utilities (both amount to ~800kB)
bundle-size
  • Added the packageManager field in the package.json instead of a dev dependency for yarn
  • Added a sideEffects: false in the package.json to reduce side-effect imports. This allowed vite to split the JsonEditor to its own chunk
  • Converted all icons to lazy imports to reduce initial bundle size

Next step would probably be jest to vitest but I have no previous experience with either.

Closes #1929

@ansmonjol
Copy link
Collaborator

Hey @ManiacMaxo, thanks a ton for you time and effort.
I tried to run it locally and could not succeed. Did you managed to make it work using all dockerized lago app? It may be an issue with our traefik proxy.

Also, few question/remark so far:

  • Is there a strong speed advantage using SWC to build? The caveats listed in the package-specially tsconfig is not resolved-would probably block me going into that direction
  • You added a comment about the sourcemap: true. I'm kinda split about this topic, as we're open source and it's convenient for potential production debugging session. I wonder what would be the speed gain not providing them during build.
  • I have some eslint errors in few files, like vite.config or svgMock and was wondering if I didn't miss anything on my side. You have them too?
  • I noticed on your first PR you removed the webpackChunkName in routes lazy load definition, but not in this PR. Are they still useful somehow? I'm not familiar to Vite but they don't seem to recommend such approach in their doc or I have not found it

Let me know!

@ManiacMaxo
Copy link
Author

Hey, I got a local docker container up and running with the API without a reverse proxy in front.

  • Regarding SWC, I've been using it in production for a few years now without any major hiccups and it has much faster HMR during dev for larger modules (there isn't much improvement in production building). For type-checking I've added running tsc before calling vite build in build script. In development this should probably be handled by the code editor. I do understand if you'd want the stability of babel.
  • Sourcemaps are enabled in the current webpack config, thats why I've enabled them here
  • The eslint errors are caused by two colliding rules: import/order and prettier/prettier
  • I've reverted the changes that removed webpackChunkName so the PR is easier to review, however it would be better if they were removed. I see there's a plugin vite-plugin-webpackchunkname that could support them if it's necessary

@ansmonjol
Copy link
Collaborator

ansmonjol commented Dec 30, 2024

Ok! I'll take time later to understand what went wrong during my setup.

  • That sounds acceptable then! We also run tsc for every main merge so we should be pretty fine.
  • Yeah and I don't think I'll change that soon. Need to dig the topic
  • Indeed and I would prefer not to disable them soooo same i'll take time on that later haha
  • I wonder if that's needed in the end. Does Vite not support this by default because it's something they handle automatically? I'm questioning the addition of an extra package that may not be much relevant. That's a detail probably and we could remove all those

Also, just to mention before it happen, In order to merge this PR I'll have to re-open it on our repo directly. Otherwise the E2E Ci job will always fail.
It's caused because inside this job we pull our api package hence needing our GH_TOKEN to be present at this time.
I'll mark you as co-author of the commit/pr of course

@ManiacMaxo
Copy link
Author

Cool, I'll just remove the webpackChunkName annotations since vite handles the lazy loading out of the box.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] Replacing webpack in favour of vite
2 participants