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

fix(web): remove warnings during the build #7035

Merged
merged 6 commits into from
Feb 12, 2024

Conversation

martabal
Copy link
Member

Remove all warnings during the build.

Copy link

cloudflare-workers-and-pages bot commented Feb 11, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0c5fe33
Status: ✅  Deploy successful!
Preview URL: https://cfcd6862.immich.pages.dev
Branch Preview URL: https://fix-remove-warning-in-web-bu.immich.pages.dev

View logs

web/src/routes/(user)/map/+page.svelte Show resolved Hide resolved
@@ -12,6 +12,9 @@ const upstream = {
};

export default defineConfig({
build: {
chunkSizeWarningLimit: 2_000_000,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2mb would be a really big chunk. It'd probably be nicer if we can figure out how to reduce chunk size instead of silencing this warning

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great, I think the problem comes from Photosphere and Map, without these modules the asset-viewer chunk size goes from 1,581.17 kB to 178.95 kB (uncompressed)

@martabal martabal force-pushed the fix/remove-warning-in-web-build branch from b46d8b7 to 8a3c900 Compare February 11, 2024 23:05
chunkSizeWarningLimit: 800,
rollupOptions: {
output: {
// optimize chunks size https://github.com/vitejs/vite/discussions/9440
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is going to create tons of small chunks which isn't really better either. Vite's defaults are pretty decent and so this feels unlikely to be an overall improvement

What I was thinking would be ideal would be if we could use a dynamic import to avoid putting some of the larger libraries in the main chunk. E.g. if I recall correctly, I think the map viewer is quite large and not loaded until you click into an image. Currently I believe it gets loaded straightaway and we pay the cost for it even if the user doesn't use it. If we do a dynamic import, we can wait to load it so that it doesn't slow down loading of the JavaScript needed for initial page load. E.g. maybe we could load it in onMount so that it doesn't block the initial JS loading, but is there in time for the user to use it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a PR to split the big chunk into two: #7057

They're both over the limit, so doesn't help in terms of removing the warning, but it should be a better user experience

I don't think there's a way to really shrink the photo sphere chunk further, so I guess we could set the limit at 600 kB to silence the warning for that one. We can probably do more with the asset viewer by splitting out the map stuff as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's nice 😃

Yeah, I saw another project doing the same thing because of maplibre. Sounds reasonable.

@alextran1502 alextran1502 merged commit c0a09d0 into main Feb 12, 2024
23 of 24 checks passed
@alextran1502 alextran1502 deleted the fix/remove-warning-in-web-build branch February 12, 2024 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants