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: avoid loading extensive @artsy libraries until necessary #14842

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

joeyAghion
Copy link
Contributor

Based on Chrome's coverage tooling, @dzucconi and I identified that the artsy-framework bundle was a little too liberal in the modules it included. Because that bundle is preloaded, that meant that all of the referenced code was being loaded on all pages, and with a high priority.

  • @artsy/to-title-case appears unused, so we removed it.
  • @artsy/icons is designed for icons to be imported individually, and occasionally gains a bloated image, so is excluded.
  • Modules like @artsy/xapp and /multienv should only be required server-side.
  • Other libs like @artsy/commerce_helpers, /dismissible, /express-reloadable, /palette-charts, and react-html-parser should only be needed in a few places, so load them there.
  • Q: What about @artsy/cohesion? It's needed for almost every route, but it also changes frequently so invalidates caching of these other libraries (which are more stable). Maybe we should add it back into this list.

Co-authored by: @dzucconi

Copy link
Member

@dzucconi dzucconi left a comment

Choose a reason for hiding this comment

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

Nice

Copy link
Member

@damassi damassi left a comment

Choose a reason for hiding this comment

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

Yess, thanks for diving into webpack!

@damassi damassi merged commit 2b90997 into main Nov 12, 2024
11 checks passed
@damassi damassi deleted the joeyAghion/coverage branch November 12, 2024 21:20
@artsy-peril artsy-peril bot mentioned this pull request Nov 12, 2024
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.

3 participants