Rename _recipe and _deployment fields on ModComponent
#9253
Conversation
| * @see ModComponentBase.modMetadata | ||
| * @since 2.0.5 | ||
| */ |
There was a problem hiding this comment.
I wonder if there already exists, or if we can create, a lint rule that requires all @<foo>s to be at the end of the JSDoc 🤔
Playwright test results
Details
Flaky testschrome › tests/pageEditor/addStarterBrick.spec.ts › Add starter brick to mod Skipped testschrome › tests/regressions/doNotCloseSidebarOnPageEditorSave.spec.ts › #8104: Do not automatically close the sidebar when saving in the Page Editor |
| /** | ||
| * @deprecated - Do not use versioned state types directly | ||
| */ | ||
| export type ModComponentBaseV4<Config extends UnknownObject = UnknownObject> = |
There was a problem hiding this comment.
Reviewer Note: type change is here
| @@ -200,9 +217,32 @@ function migrateModComponentStateV4toV5( | |||
| }; | |||
| } | |||
|
|
|||
| function migrateModComponentStateV5toV6( | |||
There was a problem hiding this comment.
Reviewer note: here's the migration
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9253 +/- ##
==========================================
+ Coverage 74.24% 75.11% +0.86%
==========================================
Files 1332 1370 +38
Lines 40817 42299 +1482
Branches 7634 7890 +256
==========================================
+ Hits 30306 31774 +1468
- Misses 10511 10525 +14 ☔ View full report in Codecov by Sentry. |
| @@ -687,4 +731,38 @@ describe("mod component state migrations", () => { | |||
| ); | |||
| }); | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
Migration test cases
| (x) => x.id, | ||
| ), | ||
| extensions: modComponents.filter((x) => !x._recipe), |
There was a problem hiding this comment.
Dropped extensions because it would always be empty
| @@ -57,7 +57,7 @@ function useDeactivateMod(): (useDeactivateConfig: Config) => Promise<void> { | |||
|
|
|||
| const modComponentIds = uniq( | |||
| [...activatedModComponents, ...modComponentFormStates] | |||
| .filter((x) => getModId(x) === modId) | |||
| .filter((x) => x.modMetadata.id === modId) | |||
There was a problem hiding this comment.
Got rid of getModId because both formState and modComponent have the same prop name now
What does this PR do?
ModComponent._recipetoModComponent.modMetadataand makes non-nullableModComponent._deploymenttoModComponent.deploymentMetadatafilterto check for presence of metadata. See Future Work for follow up workDiscussion
deploymentMetadatainstead ofdeploymentMetadata?. PR currently uses?which is the old behavior. But we could use the migration as an opportunity to ensure the property exists but the value might be undefinedRemaining Work
Future Work
standaloneModComponentRefFactoryFor more information on our expectations for the PR process, see the
code review principles doc