Skip to content

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

Merged
alextran1502 merged 6 commits into
mainfrom
fix/remove-warning-in-web-build
Feb 12, 2024
Merged

alextran1502 merged 6 commits into
mainfrom
fix/remove-warning-in-web-build

Conversation

@martabal

Copy link
Copy Markdown
Contributor

Remove all warnings during the build.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Feb 11, 2024 •

Copy link
Copy Markdown

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

Comment thread web/src/routes/(user)/map/+page.svelte
Comment thread web/vite.config.js Outdated

export default defineConfig({
build: {
chunkSizeWarningLimit: 2_000_000,

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Contributor 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
Comment thread web/vite.config.js Outdated
chunkSizeWarningLimit: 800,
rollupOptions: {
output: {
// optimize chunks size https://github.com/vitejs/vite/discussions/9440

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Collaborator

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
Copy Markdown
Contributor 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.

Comment thread web/src/routes/(user)/map/+page.svelte
@alextran1502
alextran1502 merged commit c0a09d0 into main Feb 12, 2024
@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