(fix) Improve code splitting and dependencies #1468
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR does two things: * It moves "shared" dependencies into the apps actual dependency list * It reworks how the appointments app does code splits to reduce the overall size of the app
ibacher
force-pushed
the
fix/improve-code-splitting
branch
from
9caba1f to
1d0ceaa
Compare
|
Size Change: -824 kB (-10.5%) 👏 Total Size: 7.02 MB
ℹ️ View Unchanged
|
denniskigen
approved these changes
Jan 24, 2025
|
@ibacher what's the rule of the thumb now regarding Has this changed? Should we be using |
|
So this PR originated when I was playing around with caching and noticed the appointments app was downloading a 350kB chunk of code, which seemed large (most chunks are <50kB). When I ran the analyzer there were a few minor things (it was embedding react-dom and xlsx was probably larger than it needed to be), but I noticed that Carbon was being loaded in both the "main" chunk and the 350kB one. At its heart, that's what this is fixing. So my current approximation for a rule here is: don't load something that makes heavy use of Carbon synchronously, especially "heavier" components like data tables or other things where each component uses multiple sub-components). I'm also thinking that most apps shouldn't have more than 2-3 synchronously loaded components, since that seems to be where we wind up. Maybe that can be glossed as "just load things in the sidebar synchronously"? |
|
Yeah, thanks for the context. I ask because I'd gone on to file a separate PR inspired by this one, but I feel it goes too far in the other direction as we end up with approximately one or two max synchronously loaded components per app. Those are just dashboard links. Everything else is lazy-loaded. Does that sound too far off the mark? |
No that sounds about right. I think previously, I thought that front-loading as much of an app as we could would yield better performance, but I'm no longer sure that's the case (though it probably gets you better performance on initial load). |
3 tasks
|
Ok, here's the PR. |
3 tasks
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Summary
This PR does two things:
Screenshots
Related Issue
Other