#7649: introduce platform protocol to define clear platform boundary and decouple bricks from messenger/webext APIs

[twschiller]

What does this PR do?

• Part of Define clear platform/environment boundary for bricks #7649
• The goal of this PR is to flesh out enough of the core premise that follow-on work can be more iterative
• Introduces a PlatformProtocol made available to bricks to call cross-platform affordances. Motivation:
  • Support use cases like: https://github.com/pixiebrix/pixiebrix-app/pull/4738
  • Simplify mocking boundaries
  • Reduce/eliminate accidental transitive imports across contexts
• The platform is made available to bricks in the BrickOptions
• Adds a getRequiredCapabilities method to the Brick interface to support static analysis of platform affordances
• Currently, the only platform is contentScriptPlatform. The next platform will most likely be webPagePlatform to support https://github.com/pixiebrix/pixiebrix-app/pull/4738
• Refactoring:
  • Moves contentScript-specific form and panel logic to the contentScript folder: modalDom, popoverDom, ephemeralForm, ephemeralPanel
  • Moves/renames formController, panelController, and stateController to the platform folder, because their logic doesn't depend on content-script affordances

Remaining Work

• Fix broken tests
• Add modal and sidebar APIs to protocol
• Add new files to strictNullChecks
• Add clipboard API to protocol after: #7324: Automatically use the focused document to copy to the clipboard #7635

Reviewer Tips

• Review the @/platform folder
• Review contentScriptPlatform implementation - that's also the platform passed to Jest by default (see injectRegistries test utility)
• Review the reorganized @/contentScript folder files: ephemeralForm, ephemeralPanel, modalDom, popoverDom
• The rest of the PR is primarily path/file renaming and rewriting bricks to use BrickOptions.platform. I apologize in advance for the number of files touched

Discussion

• The core idea is that the PlatformProtocol should capture useful APIs to standardize cross-platform. We're assuming a JS runtime on all environments. Examples would be: web-extension, web page, FaaS (headless node/deno), etc.
• IMO, it's OK for bricks to access the DOM APIs directly as long as they declare the "dom" capability. I don't think there's much benefit at the moment to embedding the document or window object on the platform object to enforce access via the protocol. Even in a FaaS/headless environment, we'd probably make jsdom available. (Although jsdom doesn't have access to layout)
• I decided to pass the platform to brick via BrickOptions vs. having them use the global
• In Google Sheets and AA API modules, I just used the ambient platform instead of adding a platform argument to each method. See comments in the file. Same for engineRenderer
• For bricks like SelectElement that will only ever make sense in the context of a content script, do we want to move their calls to the platform? Or just be sure to use a dynamic import and mark them with CONTENT_SCRIPT_CAPABILITIES? For ActivateTabEffect and some others I just kept the background messenger call for now

Demo

• No behavior changes

Future Work

• Finish modifying starter bricks to use the protocol. E.g., the sidebar starter brick needs to register persistent panels
• Eslint: Ban use of @/contentScript directory in @/bricks to force labeling abstraction leakage (including /api)
• Eslint: Ban use of @/background directory in @/bricks to force labeling abstraction leakage (including /api)
• Eslint: Ban use of browser/chrome global in @/bricks an error to force labeling abstraction leakage
• Eslint: Ban @/utils/notify in the runtime folder to force labeling abstraction leakage
• Add logging and notification concerns to the platform. Mods currently use the background worker logger
• Use capability declarations to statically analyze if a mod can be run on a certain platform

Checklist

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

[codecov]

Codecov Report

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

Comparison is base (048255a) 72.77% compared to head (a319a58) 72.52%.

❗ Current head a319a58 differs from pull request most recent head 06a5889. Consider uploading reports for the commit 06a5889 to get more accurate results

Files	Patch %	Lines
src/platform/platformProtocol.ts	12.19%	36 Missing ⚠️
src/contentScript/ephemeralPanel.ts	83.72%	14 Missing ⚠️
src/contentScript/contentScriptPlatform.ts	83.56%	12 Missing ⚠️
src/bricks/transformers/httpGet.ts	0.00%	8 Missing ⚠️
src/contrib/hubspot/upsert.ts	0.00%	8 Missing ⚠️
src/contrib/pipedrive/create.ts	0.00%	8 Missing ⚠️
src/contrib/slack/message.ts	0.00%	8 Missing ⚠️
src/bricks/effects/redirectPage.ts	0.00%	7 Missing ⚠️
src/bricks/transformers/brickFactory.ts	12.50%	7 Missing ⚠️
src/sidebar/sidebarPlatform.ts	0.00%	7 Missing ⚠️
... and 42 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7650      +/-   ##
==========================================
- Coverage   72.77%   72.52%   -0.25%     
==========================================
  Files        1251     1259       +8     
  Lines       39018    39262     +244     
  Branches     7319     7326       +7     
==========================================
+ Hits        28396    28476      +80     
- Misses      10622    10786     +164     

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

[twschiller]

@grahamlangford I think this is good for another review

[twschiller]

Let's create an epic with these issues so we can start prioritizing the work. Especially the lint rules to ban imports so that we don't start violating it in the interim.

Created here: #7680

[grahamlangford]

Still having issues with the tooltipController test.

[github-actions]

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.