#6581: lazy load page script to reduce memory footprint #6772

twschiller · 2023-10-30T18:52:26Z

What does this PR do?

Part of Investigate delayed execution of bricks in resource-starved browsers #6581
The page script stat size is 868kb
The page script is only used by the Page Editor and a few rare bricks. However, lifecycle.ts was loading the Page Editor on every frame, and the injection was blocking mod startup
Refactors legacy messenger utils out of pageScript folder to @/utils/legacyMessengerUtils
Fixes a circular dependency between selector inference and selector hints
Extracts inference method that calls the page script into: src/contentScript/pageEditor/inferSingleElementSelector.ts

Remaining Work

Fix browser-polyfill check

Future Work

Drop the deprecated framework detection for the element (which requires the page script, so therefore needed to be split from the other inference code)

Checklist

Add tests: TODO: check to see if we have existing Rainforest QA tests
New files added to src/tsconfig.strictNullChecks.json (if possible): N/A
Designate a primary reviewer: @grahamlangford

twschiller · 2023-10-30T18:52:48Z

src/pageScript/messenger/pigeon.ts

@@ -47,6 +51,16 @@ function assertCustomEvent(
  }
 }

+const injectPageScriptOnce = once(async (): Promise<void> => {


Moved from lifecycle.ts

twschiller · 2023-10-30T18:52:56Z

src/contentScript/lifecycle.ts

@@ -89,15 +88,6 @@ const _navigationListeners = new Set<() => void>();

 const WAIT_LOADED_INTERVAL_MS = 25;

-const injectPageScriptOnce = once(async (): Promise<void> => {


Moved to pigeon.ts

Looks like it's in src/pageScript/messenger/sender.ts

Looks like it's in src/pageScript/messenger/sender.ts

Move to src/pageScript/messenger/sender.ts 😄

codecov · 2023-10-30T19:08:10Z

Codecov Report

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

Comparison is base (ed759a5) 70.05% compared to head (5793ae9) 70.03%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6772      +/-   ##
==========================================
- Coverage   70.05%   70.03%   -0.02%     
==========================================
  Files        1188     1193       +5     
  Lines       36936    36949      +13     
  Branches     6937     6937              
==========================================
+ Hits        25875    25879       +4     
- Misses      11061    11070       +9

Files	Coverage Δ
src/background/contentScript.ts	`96.72% <100.00%> (ø)`
src/background/externalProtocol.ts	`0.00% <ø> (ø)`
src/background/messenger/external/api.ts	`0.00% <ø> (ø)`
src/contentScript/externalProtocol.ts	`0.00% <ø> (ø)`
src/contentScript/lifecycle.ts	`67.14% <100.00%> (-0.78%)`	⬇️
src/contentScript/pageEditor.ts	`0.00% <ø> (ø)`
src/contentScript/pageEditor/elementPicker.ts	`67.07% <100.00%> (+0.13%)`	⬆️
...geEditor/tabs/effect/useDocumentPreviewRunBlock.ts	`55.07% <ø> (ø)`
src/pageScript/frameworks.ts	`0.00% <ø> (ø)`
src/pageScript/messenger/api.ts	`100.00% <100.00%> (ø)`
... and 9 more

... and 1 file with indirect coverage changes

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

fregante

I don't immediately see what's importing the polyfill. expectContext does not use browser

https://github.com/pixiebrix/pixiebrix-extension/blob/main/src/utils/expectContext.ts

fregante · 2023-10-30T22:21:26Z

src/utils/legacyMessengerUtils.ts

+
+/** @file The first messenger before webext-messenger. Deprecated, see https://github.com/pixiebrix/webext-messenger/issues/5 */
+
+export interface RemoteProcedureCallRequest<TMeta extends Meta = Meta>


Moving these utils to a separate files opens them to being mistakenly imported since they're not individually deprecated.

👍 I'll deprecate the whole file

github-actions · 2023-10-31T01:16:26Z

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 requested review from fregante and grahamlangford October 30, 2023 18:52

twschiller self-assigned this Oct 30, 2023

twschiller commented Oct 30, 2023

View reviewed changes

vercel bot deployed to Preview October 30, 2023 20:37 View deployment

fregante reviewed Oct 30, 2023

View reviewed changes

twschiller changed the title ~~#6581: lazy load page script~~ #6581: lazy load page script to reduce memory footprint Oct 30, 2023

vercel bot deployed to Preview October 31, 2023 00:21 View deployment

vercel bot deployed to Preview October 31, 2023 00:33 View deployment

twschiller added 3 commits October 30, 2023 20:36

#6581: lazy load page script

298d7dd

#6581: split up sender/receiver

320daad

#6581: split cycle

61bc4fb

twschiller force-pushed the feature/6581-lazy-load-page-script branch from 660f1b6 to 61bc4fb Compare October 31, 2023 00:38

vercel bot deployed to Preview October 31, 2023 00:40 View deployment

Fix lint

5793ae9

twschiller added performance developer experience labels Oct 31, 2023

vercel bot deployed to Preview October 31, 2023 00:46 View deployment

grahamlangford approved these changes Oct 31, 2023

View reviewed changes

twschiller merged commit aefc3e5 into main Oct 31, 2023
13 checks passed

twschiller deleted the feature/6581-lazy-load-page-script branch October 31, 2023 01:24

twschiller mentioned this pull request Oct 31, 2023

Drop front-end framework detection code #6776

Merged

3 tasks

grahamlangford added this to the 1.8.2 milestone Oct 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#6581: lazy load page script to reduce memory footprint #6772

#6581: lazy load page script to reduce memory footprint #6772

twschiller commented Oct 30, 2023 •

edited

Loading

twschiller Oct 30, 2023

twschiller Oct 30, 2023

grahamlangford Oct 30, 2023

twschiller Oct 30, 2023

codecov bot commented Oct 30, 2023 •

edited

Loading

fregante left a comment

fregante Oct 30, 2023

twschiller Oct 30, 2023

github-actions bot commented Oct 31, 2023

		@@ -89,15 +88,6 @@ const _navigationListeners = new Set<() => void>();

		const WAIT_LOADED_INTERVAL_MS = 25;

		const injectPageScriptOnce = once(async (): Promise<void> => {


		/** @file The first messenger before webext-messenger. Deprecated, see https://github.com/pixiebrix/webext-messenger/issues/5 */

		export interface RemoteProcedureCallRequest<TMeta extends Meta = Meta>

#6581: lazy load page script to reduce memory footprint #6772

#6581: lazy load page script to reduce memory footprint #6772

Conversation

twschiller commented Oct 30, 2023 • edited Loading

What does this PR do?

Remaining Work

Future Work

Checklist

twschiller Oct 30, 2023

Choose a reason for hiding this comment

twschiller Oct 30, 2023

Choose a reason for hiding this comment

grahamlangford Oct 30, 2023

Choose a reason for hiding this comment

twschiller Oct 30, 2023

Choose a reason for hiding this comment

codecov bot commented Oct 30, 2023 • edited Loading

Codecov Report

fregante left a comment

Choose a reason for hiding this comment

fregante Oct 30, 2023

Choose a reason for hiding this comment

twschiller Oct 30, 2023

Choose a reason for hiding this comment

github-actions bot commented Oct 31, 2023

twschiller commented Oct 30, 2023 •

edited

Loading

codecov bot commented Oct 30, 2023 •

edited

Loading