Strict null checks -- 90% #8863

grahamlangford · 2024-07-16T20:27:45Z

What does this PR do?

Focuses on Page Editor files for strict null checks
Raises coverage to 90%
1781/1978 files checked

For more information on our expectations for the PR process, see the
code review principles doc

github-actions · 2024-07-16T20:44:13Z

Playwright test results

105 passed
1 flaky
4 skipped

Details

Open report ↗︎
110 tests across 36 suites
8 minutes, 8 seconds
42e7b9e
For more information on how to debug and view this report, see our readme

Flaky tests

edgeSetup › setup/unaffiliated.setup.ts › authenticate with unaffiliated user

Skipped tests

chrome › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor
edge › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor
chrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options
edge › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options

codecov · 2024-07-16T21:07:45Z

Codecov Report

Attention: Patch coverage is 87.95181% with 10 lines in your changes missing coverage. Please review.

Project coverage is 74.38%. Comparing base (8318d74) to head (42e7b9e).
Report is 95 commits behind head on main.

Files	Patch %	Lines
src/pageEditor/fields/AssignModVariableOptions.tsx	25.00%	3 Missing ⚠️
...geEditor/tabs/effect/useDocumentPreviewRunBlock.ts	71.42%	2 Missing ⚠️
...pageEditor/documentBuilder/hooks/useMoveElement.ts	50.00%	1 Missing ⚠️
src/pageEditor/fields/AlertOptions.tsx	50.00%	1 Missing ⚠️
src/pageEditor/fields/RemoteMethodOptions.tsx	50.00%	1 Missing ⚠️
src/pageEditor/hooks/useSaveMod.ts	83.33%	1 Missing ⚠️
.../pageEditor/modals/addBrickModal/AddBrickModal.tsx	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8863      +/-   ##
==========================================
+ Coverage   74.24%   74.38%   +0.13%     
==========================================
  Files        1332     1340       +8     
  Lines       40817    41198     +381     
  Branches     7634     7710      +76     
==========================================
+ Hits        30306    30646     +340     
- Misses      10511    10552      +41

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

src/pageEditor/documentBuilder/hooks/useMoveElement.ts

src/pageEditor/documentBuilder/preview/elementsPreview/Card.tsx

src/pageEditor/hooks/useSaveMod.ts

BLoe · 2024-07-16T21:40:30Z

src/pageEditor/hooks/useSaveMod.ts

@@ -149,13 +152,17 @@ function useSaveMod(): ModSaver {
      (x) => x.name === newMod.metadata.id,
    )?.id;

+    assertNotNullish(packageId, "Package ID is required to upsert a mod");


Same question, are we sure we want this function to throw an error if the callback is used before the RTK Query request finishes, as opposed to silently returning early / no-op?

In this case, I think we do want an error, because editablePackages would already be loaded by the time we get here. So the packageId generally can't be found

src/pageEditor/modals/addBrickModal/addBrickModalHelpers.ts

BLoe

Just had a few nit picks 👍

github-actions · 2024-07-16T21:54:42Z

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.

grahamlangford added 25 commits July 16, 2024 13:05

./pageEditor/documentBuilder/hooks/useMoveElement.ts

65dec24

auto-add

4de0607

./extensionConsole/pages/activateMod/useModPermissions.ts

dc36dc9

auto-add

a4afdc6

./pageEditor/documentBuilder/preview/elementsPreview/Card.tsx

38dc464

./pageEditor/documentBuilder/preview/elementsPreview/Container.tsx

50f1f0d

./pageEditor/documentBuilder/preview/elementsPreview/Image.tsx

536568e

./pageEditor/documentBuilder/preview/getPreviewComponentDefinition.tsx

948bd0c

auto-add

88c2af5

./pageEditor/documentBuilder/preview/ElementPreview.tsx

23cf913

./pageEditor/hooks/useSaveMod.ts

649cb02

./pageEditor/documentBuilder/edit/MoveElement.tsx

ee0e9a8

./pageEditor/documentBuilder/edit/RemoveElement.tsx

23e45b7

./pageEditor/fields/FormRendererOptions.tsx

20d9922

./pageEditor/fields/RemoteMethodOptions.tsx

71824e7

./pageEditor/fields/AlertOptions.tsx

c93b140

./pageEditor/fields/AssignModVariableOptions.tsx

7129862

./pageEditor/modals/addBrickModal/useAddBrick.ts

4a64430

./pageEditor/tabs/editTab/dataPanel/StarterBrickDataPanel.tsx

01505c4

./pageEditor/tabs/effect/useDocumentPreviewRunBlock.ts

7fb3391

./pageEditor/tabs/modOptionsDefinitions/ModOptionsDefinitionEditor.tsx

0352d60

./pageEditor/tabs/modOptionsValues/ModOptionsValuesEditor.tsx

80f4fe1

auto-add

712034c

./pageEditor/documentBuilder/edit/DocumentOptions.tsx

89da96e

./pageEditor/modals/addBrickModal/AddBrickModal.tsx

cfb5e21

grahamlangford requested review from BLoe, fungairino and mnholtz July 16, 2024 20:28

grahamlangford self-assigned this Jul 16, 2024

grahamlangford added this to the 2.0.6 milestone Jul 16, 2024

twschiller added the developer experience label Jul 16, 2024

fix tests

5b31170

vercel bot deployed to Preview July 16, 2024 21:01 View deployment

Merge branch 'main' into strict-null-checks-16-jul

4aaeeac

BLoe reviewed Jul 16, 2024

View reviewed changes

src/pageEditor/documentBuilder/hooks/useMoveElement.ts Outdated Show resolved Hide resolved

vercel bot deployed to Preview July 16, 2024 21:29 View deployment

BLoe reviewed Jul 16, 2024

View reviewed changes

src/pageEditor/documentBuilder/preview/elementsPreview/Card.tsx Outdated Show resolved Hide resolved

BLoe reviewed Jul 16, 2024

View reviewed changes

src/pageEditor/hooks/useSaveMod.ts Outdated Show resolved Hide resolved

BLoe reviewed Jul 16, 2024

View reviewed changes

src/pageEditor/modals/addBrickModal/addBrickModalHelpers.ts Outdated Show resolved Hide resolved

BLoe approved these changes Jul 16, 2024

View reviewed changes

pr-related changes

0ad1ffd

vercel bot deployed to Preview July 17, 2024 12:12 View deployment

fix failing e2e test; rename block -> brick

42e7b9e

vercel bot deployed to Preview July 17, 2024 12:55 View deployment

grahamlangford merged commit ecd21ea into main Jul 17, 2024
22 checks passed

grahamlangford deleted the strict-null-checks-16-jul branch July 17, 2024 13:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strict null checks -- 90% #8863

Strict null checks -- 90% #8863

grahamlangford commented Jul 16, 2024

github-actions bot commented Jul 16, 2024 •

edited

Loading

codecov bot commented Jul 16, 2024 •

edited

Loading

BLoe Jul 16, 2024 •

edited

Loading

grahamlangford Jul 17, 2024

BLoe left a comment

github-actions bot commented Jul 16, 2024

Strict null checks -- 90% #8863

Strict null checks -- 90% #8863

Conversation

grahamlangford commented Jul 16, 2024

What does this PR do?

github-actions bot commented Jul 16, 2024 • edited Loading

Playwright test results

Details

codecov bot commented Jul 16, 2024 • edited Loading

Codecov Report

BLoe Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

grahamlangford Jul 17, 2024

Choose a reason for hiding this comment

BLoe left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 16, 2024

github-actions bot commented Jul 16, 2024 •

edited

Loading

codecov bot commented Jul 16, 2024 •

edited

Loading

BLoe Jul 16, 2024 •

edited

Loading