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

#4000: Dynamically load brick UI to reduce bundle size #6777

Merged
merged 7 commits into from Oct 31, 2023

Conversation

twschiller
Copy link
Contributor

@twschiller twschiller commented Oct 31, 2023

What does this PR do?

  • Part of Reduce memory footprint of sidebar #4000
  • Dynamically import brick UI modules to: 1) eliminate bundling code in contexts that can never run the code, 2) reduce the default footprint of the content script
  • Dynamically import Rollbar so it's not included outside the background page

Refactored Imports

  • Dynamically load RJSF in the customForm brick
  • Dynamically load the Quick Bar module
  • Dynamically load iframe-resize
  • Dynamically load font-awesome
    • Remove static font-awesome import from tourController
  • Dynamically load rollbar

Discussion

  • Refactoring some methods to be async to split out UI modules is a bit awkward/bad DX. But the bundle size benefit makes the refactor necessary.

Demo

Background Page without font-awesome/rjsf/etc.
image

Future Work

  • Add CI checks to fail CI if these modules make their way back into the entry points.
    • Also it would be nice to track entry point size changes as part of PR
  • Most impactful?: Figure out why font-awesome tree-shaking is broken (I suspect it will still need to be in the entry point of sidebar/etc., but it should be significantly smaller), or switch to deep imports: https://fontawesome.com/docs/apis/javascript/tree-shaking Update: tree-shaking is working as expected. The problem is we include font-awesome's icon packs for dynamic Mod icon lookup:
    export async function fetchFortAwesomeIcon(
  • Figure out why lodash tree-shaking is broken
  • Eliminate react-dom + formik from the background page

Checklist

  • Add tests: N/A
  • New files added to src/tsconfig.strictNullChecks.json (if possible)
  • Designate a primary reviewer: @grahamlangford

@codecov
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (aefc3e5) 70.03% compared to head (fd24e22) 69.98%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6777      +/-   ##
==========================================
- Coverage   70.03%   69.98%   -0.05%     
==========================================
  Files        1193     1195       +2     
  Lines       36967    36976       +9     
  Branches     6939     6939              
==========================================
- Hits        25888    25877      -11     
- Misses      11079    11099      +20     
Files Coverage Δ
src/bricks/effects/highlightText.ts 91.42% <100.00%> (ø)
src/bricks/renderers/customForm.ts 56.17% <100.00%> (ø)
src/bricks/renderers/document.ts 80.00% <ø> (ø)
...transformers/temporaryInfo/DisplayTemporaryInfo.ts 84.25% <100.00%> (ø)
src/components/quickBar/QuickBarApp.tsx 93.54% <100.00%> (+0.44%) ⬆️
src/components/quickBar/quickBarRegistry.ts 92.15% <100.00%> (-0.16%) ⬇️
src/starterBricks/quickBarExtension.tsx 54.83% <100.00%> (ø)
src/starterBricks/quickBarProviderExtension.tsx 75.96% <100.00%> (ø)
src/starterBricks/tourActionIcon.tsx 100.00% <100.00%> (ø)
src/starterBricks/tourController.ts 90.00% <100.00%> (ø)
... and 5 more

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@twschiller twschiller changed the title #4000: [WIP] Dynamically load brick UI #4000: [WIP] Dynamically load brick UI to reduce bundle size Oct 31, 2023
@twschiller twschiller changed the title #4000: [WIP] Dynamically load brick UI to reduce bundle size #4000: Dynamically load brick UI to reduce bundle size Oct 31, 2023
Copy link

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.

@twschiller twschiller merged commit 35bf764 into main Oct 31, 2023
14 checks passed
@twschiller twschiller deleted the feature/lazy-load-ui branch October 31, 2023 13:37
@grahamlangford grahamlangford added this to the 1.8.2 milestone Oct 31, 2023
@@ -37,6 +36,11 @@ class ToggleQuickbarEffect extends EffectABC {
inputSchema: Schema = propertiesToSchema({}, []);

async effect(): Promise<void> {
const { toggleQuickBar } = await import(
/* webpackChunkName: "quickBarApp" */
"@/components/quickBar/QuickBarApp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

In these PRs make sure that these files are exclusively imported this way, because import() alone doesn't guarantee that.

See

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.

None yet

3 participants