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

chore(vite): use different configs for admin and site; don't split assets into chunks #3440

Merged
merged 6 commits into from
Apr 8, 2024

Conversation

marcelgerber
Copy link
Member

@marcelgerber marcelgerber commented Apr 2, 2024

I didn't want to go down this path necessarily, but:

  • The splitting of our js/css into owid, common, admin is of dubious value, since there isn't much of an optimization since admin is used by very few people.
  • In our bundlewatch checks, we sometimes have changes which move code from one bundle to the other; these always cause spurious bundlewatch errors then.
  • But on the other hand, Vite/Rollup doesn't make it easy not to generate shared chunks between modules.
  • ... so we need to resort to creating two Vite configs, and building both of them separately.

This fixes #2904.
It'll also probably speed up pageloads by a tiny bit.


This doesn't change anything about the dev setup, which is still fully served by a single Vite dev server.
For production builds, we produce two types of outputs into two directories, /assets and /assets-admin:
CleanShot 2024-04-03 at 14 37 22

These are now entirely self-contained and independent of each other.
This also means that the owid.mjs file no longer contains any import statements, which is what fixes #2904.


TODOs

@marcelgerber marcelgerber changed the title chore(vite): use different configs for admin and site chore(vite): use different configs for admin and site; don't split assets into chunks Apr 2, 2024
@marcelgerber marcelgerber force-pushed the vite-separate-builds branch from b9e7424 to 548522d Compare April 3, 2024 09:36
@marcelgerber
Copy link
Member Author

Just confirmed that this actually fixes #2904:

This branch is deployed to https://vite-separate-builds.owid-staging.pages.dev/ on CF Pages (some thinks aren't loading correctly because the baseUrl was wrongly set - sorry for that...)

I then archived a grapher page on there: https://web.archive.org/web/20240403094959/https://vite-separate-builds.owid-staging.pages.dev/grapher/life-expectancy

And it works 🎉

@marcelgerber marcelgerber force-pushed the vite-separate-builds branch from 02f438a to 06e7e0f Compare April 3, 2024 12:23
@marcelgerber marcelgerber marked this pull request as ready for review April 3, 2024 12:39
@marcelgerber marcelgerber requested a review from danyx23 April 3, 2024 12:39
@marcelgerber
Copy link
Member Author

Main things to take care of (and which I've hopefully checked enough are):

  • That /assets-admin is probably served on the admin
  • That both types of assets are correctly copied to their respective places
  • That VITE_PREVIEW=true mode works well
  • That staging servers work well

@danyx23
Copy link
Contributor

danyx23 commented Apr 3, 2024

Amazing that you managed to find a solution for #2904! I think it would be best to talk this through with you and Ike and then I'll gladly play around with it. I'll send you a message to find a slot.

@danyx23
Copy link
Contributor

danyx23 commented Apr 5, 2024

Hi Marcel - unfortunately I didn't have time to check this anymore. Conceptually it's good but maybe it would be good if @ikesau could have a look at it before you merge it next week?

@marcelgerber marcelgerber merged commit fd61589 into master Apr 8, 2024
22 checks passed
@marcelgerber marcelgerber deleted the vite-separate-builds branch April 8, 2024 11:24
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.

OWID's JS code does not run when captured in the Internet Archive
2 participants