Refactor: Making TaskStore a regular context #3732
Conversation
… using zustand. It is only really used to override things, and the pattern we used to wait until things were set in zustand clearly indicated this was the wrong approach.
olemartinorg
added
kind/other
📝 WalkthroughWalkthroughReplaces the old TaskStore context with a new TaskOverrides React context and hook. Removes TaskStoreProvider usage throughout the app and tests. Updates many consumers to read overrides via useTaskOverrides, adds SubformOverrideWrapper, and shifts layout-set resolution to a URL-driven hook. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)src/features/devtools/components/LayoutInspector/LayoutInspector.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/datamodel/DataModelsProvider.tsx (1)
214-225: Align layout-set source between DataModelsProvider and LayoutsContextDataModelsLoader stores layoutSetId from useCurrentLayoutSetId, while LayoutsContext selects layouts via useLayoutSetId (which prefers TaskOverrides / useNavigationParam('taskId') before falling back) — they can diverge and cause spurious 'layout-set-change' loaders. Use the same source (e.g., call useLayoutSetId or derive the id from the layouts used) when calling setDataTypes in src/features/datamodel/DataModelsProvider.tsx.
🧹 Nitpick comments (7)
src/core/contexts/TaskOverrides.tsx (3)
13-13: Remove unused Omit key ('depth').
depthisn’t part ofTaskOverridesContext; dropping theOmitreduces confusion.-type Props = PropsWithChildren & Omit<TaskOverridesContext, 'depth'>; +type Props = PropsWithChildren & TaskOverridesContext;
11-11: Add a displayName for easier React DevTools debugging.-const Context = createContext<TaskOverridesContext>({}); +const Context = createContext<TaskOverridesContext>({}); +Context.displayName = 'TaskOverridesContext';
1-2: Memoize provider value to avoid unnecessary re-renders of consumers.-import React, { createContext, useContext } from 'react'; +import React, { createContext, useContext, useMemo } from 'react'; @@ - return ( - <Context.Provider - value={{ - taskId: overrides.taskId ?? parentContext.taskId, - dataModelType: overrides.dataModelType ?? parentContext.dataModelType, - dataModelElementId: overrides.dataModelElementId ?? parentContext.dataModelElementId, - layoutSetId: overrides.layoutSetId ?? parentContext.layoutSetId, - }} - > + const value = useMemo( + () => ({ + taskId: overrides.taskId ?? parentContext.taskId, + dataModelType: overrides.dataModelType ?? parentContext.dataModelType, + dataModelElementId: overrides.dataModelElementId ?? parentContext.dataModelElementId, + layoutSetId: overrides.layoutSetId ?? parentContext.layoutSetId, + }), + [ + overrides.taskId, + overrides.dataModelType, + overrides.dataModelElementId, + overrides.layoutSetId, + parentContext.taskId, + parentContext.dataModelType, + parentContext.dataModelElementId, + parentContext.layoutSetId, + ], + ); + return ( + <Context.Provider value={value}> {children} </Context.Provider> );Also applies to: 17-24
src/layout/FileUpload/FileUploadTable/FileTableRow.tsx (1)
43-46: Rename typo and simplify boolean derivation.Use a single boolean and fix “Overriden” → “Overridden”.
- const overriddenTaskId = useTaskOverrides()?.taskId; - - const hasOverridenTaskId = !!overriddenTaskId; + const hasOverriddenTaskId = Boolean(useTaskOverrides()?.taskId); @@ - style={hasOverridenTaskId ? { padding: '8px 0' } : {}} + style={hasOverriddenTaskId ? { padding: '8px 0' } : {}}Also applies to: 81-81
src/features/attachments/StoreAttachmentsInNode.tsx (1)
92-103: Use nullish coalescing for readabilitySmall readability tweak; keeps behavior identical.
- const overriddenTaskId = useTaskOverrides()?.taskId; + const overriddenTaskId = useTaskOverrides()?.taskId; @@ - const mappedAttachments = useMemoDeepEqual(() => { - const taskId = overriddenTaskId ? overriddenTaskId : currentTask; - - return mapAttachments(indexedId, baseId, data, application, taskId, nodeData); - }, [indexedId, baseId, data, application, currentTask, nodeData, overriddenTaskId]); + const mappedAttachments = useMemoDeepEqual(() => { + const taskId = overriddenTaskId ?? currentTask; + return mapAttachments(indexedId, baseId, data, application, taskId, nodeData); + }, [indexedId, baseId, data, application, currentTask, nodeData, overriddenTaskId]);src/layout/Summary2/CommonSummaryComponents/EditButton.tsx (1)
69-85: Tighten conditionalsMinor simplification to avoid redundant checks.
- const overrides = useTaskOverrides(); - const overriddenTaskId = overrides?.taskId; - const overriddenDataElementId = overrides?.dataModelElementId; + const overrides = useTaskOverrides(); + const overriddenTaskId = overrides?.taskId; + const overriddenDataElementId = overrides?.dataModelElementId; @@ - if (pdfModeActive || (overriddenTaskId && overriddenTaskId?.length > 0)) { + if (pdfModeActive || overriddenTaskId?.length) { return null; }src/test/renderWithProviders.tsx (1)
481-505: Optional: Add an easy path to inject TaskOverrides in testsDefaultProviders/MinimalProviders in src/test/renderWithProviders.tsx do not provide TaskOverrides, and many components call useTaskOverrides (e.g. src/layout/Subform/SubformWrapper.tsx, src/layout/Summary2/CommonSummaryComponents/EditButton.tsx, src/features/instance/useProcessTaskId.ts). Add an optional wrapper prop or an overrides argument to the test render helpers (or accept an overrides object in DefaultProviders/MinimalProviders) so tests can pass { taskId, dataModelType, dataModelElementId, layoutSetId } when needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/core/contexts/TaskOverrides.tsx(1 hunks)src/core/contexts/taskStoreContext.tsx(0 hunks)src/features/attachments/StoreAttachmentsInNode.tsx(2 hunks)src/features/datamodel/DataModelsProvider.tsx(2 hunks)src/features/datamodel/useBindingSchema.tsx(3 hunks)src/features/form/layout/LayoutsContext.tsx(2 hunks)src/features/form/layoutSets/useCurrentLayoutSet.ts(2 hunks)src/features/instance/useProcessTaskId.ts(1 hunks)src/features/instantiate/containers/InstantiationContainer.tsx(1 hunks)src/features/instantiate/selection/InstanceSelection.tsx(3 hunks)src/index.tsx(1 hunks)src/layout/FileUpload/FileUploadTable/FileTableRow.tsx(2 hunks)src/layout/Subform/SubformWrapper.tsx(4 hunks)src/layout/Subform/Summary/SubformSummaryComponent2.tsx(5 hunks)src/layout/Subform/index.tsx(1 hunks)src/layout/Summary2/CommonSummaryComponents/EditButton.tsx(2 hunks)src/layout/Summary2/SummaryComponent2/SummaryComponent2.tsx(1 hunks)src/layout/Summary2/SummaryComponent2/TaskSummaryWrapper.tsx(2 hunks)src/test/renderWithProviders.tsx(2 hunks)src/utils/layout/all.test.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/core/contexts/taskStoreContext.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid usinganyand unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options viaqueryOptions
Files:
src/features/form/layoutSets/useCurrentLayoutSet.tssrc/features/instantiate/containers/InstantiationContainer.tsxsrc/features/instantiate/selection/InstanceSelection.tsxsrc/layout/FileUpload/FileUploadTable/FileTableRow.tsxsrc/core/contexts/TaskOverrides.tsxsrc/index.tsxsrc/features/datamodel/useBindingSchema.tsxsrc/layout/Summary2/SummaryComponent2/SummaryComponent2.tsxsrc/layout/Summary2/SummaryComponent2/TaskSummaryWrapper.tsxsrc/layout/Subform/Summary/SubformSummaryComponent2.tsxsrc/features/instance/useProcessTaskId.tssrc/test/renderWithProviders.tsxsrc/utils/layout/all.test.tsxsrc/layout/Summary2/CommonSummaryComponents/EditButton.tsxsrc/layout/Subform/SubformWrapper.tsxsrc/features/attachments/StoreAttachmentsInNode.tsxsrc/features/form/layout/LayoutsContext.tsxsrc/layout/Subform/index.tsxsrc/features/datamodel/DataModelsProvider.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In tests, use
renderWithProvidersfromsrc/test/renderWithProviders.tsxto supply required form layout context
Files:
src/utils/layout/all.test.tsx
src/layout/*/{config.ts,Component.tsx,index.tsx,config.generated.ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Layout components must follow the standardized structure:
config.ts,Component.tsx,index.tsx, and include generated types inconfig.generated.ts
Files:
src/layout/Subform/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
PR: Altinn/app-frontend-react#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to **/*.test.{ts,tsx} : In tests, use `renderWithProviders` from `src/test/renderWithProviders.tsx` to supply required form layout context
Applied to files:
src/features/form/layoutSets/useCurrentLayoutSet.tssrc/layout/Summary2/SummaryComponent2/TaskSummaryWrapper.tsxsrc/layout/Subform/Summary/SubformSummaryComponent2.tsxsrc/test/renderWithProviders.tsxsrc/utils/layout/all.test.tsxsrc/layout/Subform/SubformWrapper.tsxsrc/features/form/layout/LayoutsContext.tsxsrc/layout/Subform/index.tsx
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
PR: Altinn/app-frontend-react#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to src/features/*/Provider.tsx : Place feature-specific context providers as `Provider.tsx` within each feature directory under `src/features/`
Applied to files:
src/index.tsxsrc/test/renderWithProviders.tsx
🧬 Code graph analysis (18)
src/features/form/layoutSets/useCurrentLayoutSet.ts (1)
src/core/contexts/TaskOverrides.tsx (1)
useTaskOverrides(31-31)
src/features/instantiate/containers/InstantiationContainer.tsx (2)
src/core/ui/RenderStart.tsx (1)
RenderStart(22-31)src/components/ReadyForPrint.tsx (1)
ReadyForPrint(21-52)
src/features/instantiate/selection/InstanceSelection.tsx (2)
src/features/instantiate/selection/ActiveInstancesProvider.tsx (1)
ActiveInstancesProvider(43-47)src/components/presentation/Presentation.tsx (1)
PresentationComponent(34-99)
src/layout/FileUpload/FileUploadTable/FileTableRow.tsx (1)
src/core/contexts/TaskOverrides.tsx (1)
useTaskOverrides(31-31)
src/core/contexts/TaskOverrides.tsx (1)
src/core/contexts/context.tsx (1)
createContext(45-89)
src/index.tsx (8)
src/features/applicationMetadata/ApplicationMetadataProvider.tsx (1)
ApplicationMetadataProvider(59-65)src/features/formData/FormDataReaders.tsx (1)
GlobalFormDataReadersProvider(106-158)src/features/language/LanguageProvider.tsx (1)
SetShouldFetchAppLanguages(90-102)src/features/party/PartiesProvider.tsx (1)
PartyProvider(168-180)src/core/auth/KeepAliveProvider.tsx (1)
KeepAliveProvider(48-55)src/core/errorHandling/DisplayErrorProvider.tsx (1)
DisplayErrorProvider(9-17)src/core/contexts/processingContext.tsx (1)
ProcessingProvider(14-24)src/queries/partyPrefetcher.ts (1)
PartyPrefetcher(8-15)
src/features/datamodel/useBindingSchema.tsx (1)
src/core/contexts/TaskOverrides.tsx (1)
useTaskOverrides(31-31)
src/layout/Summary2/SummaryComponent2/SummaryComponent2.tsx (2)
src/layout/Summary2/summaryStoreContext.tsx (1)
Summary2StoreProvider(16-27)src/layout/Summary2/SummaryComponent2/TaskSummaryWrapper.tsx (1)
TaskSummaryWrapper(14-28)
src/layout/Summary2/SummaryComponent2/TaskSummaryWrapper.tsx (2)
src/core/contexts/TaskOverrides.tsx (1)
TaskOverrides(14-29)src/features/form/FormContext.tsx (1)
FormProvider(43-86)
src/layout/Subform/Summary/SubformSummaryComponent2.tsx (2)
src/core/contexts/TaskOverrides.tsx (1)
TaskOverrides(14-29)src/layout/Summary2/SummaryComponent2/LayoutSetSummary.tsx (1)
LayoutSetSummary(13-33)
src/features/instance/useProcessTaskId.ts (1)
src/core/contexts/TaskOverrides.tsx (1)
useTaskOverrides(31-31)
src/utils/layout/all.test.tsx (1)
src/layout/Subform/SubformWrapper.tsx (1)
SubformWrapper(16-22)
src/layout/Summary2/CommonSummaryComponents/EditButton.tsx (1)
src/core/contexts/TaskOverrides.tsx (1)
useTaskOverrides(31-31)
src/layout/Subform/SubformWrapper.tsx (2)
src/features/form/FormContext.tsx (1)
FormProvider(43-86)src/core/contexts/TaskOverrides.tsx (1)
TaskOverrides(14-29)
src/features/attachments/StoreAttachmentsInNode.tsx (1)
src/core/contexts/TaskOverrides.tsx (1)
useTaskOverrides(31-31)
src/features/form/layout/LayoutsContext.tsx (1)
src/core/contexts/TaskOverrides.tsx (1)
useTaskOverrides(31-31)
src/layout/Subform/index.tsx (1)
src/layout/Subform/SubformWrapper.tsx (3)
SubformWrapper(16-22)SubformForm(24-32)RedirectBackToMainForm(34-43)
src/features/datamodel/DataModelsProvider.tsx (1)
src/core/contexts/TaskOverrides.tsx (1)
useTaskOverrides(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Type-checks, eslint, unit tests and SonarCloud
- GitHub Check: Install
- GitHub Check: Analyze (javascript)
🔇 Additional comments (27)
src/layout/Summary2/SummaryComponent2/SummaryComponent2.tsx (1)
31-35: LGTM: Provider stack simplified correctly.Wrapping with
Summary2StoreProviderand usingTaskSummaryWrapperfor overrides matches the new context approach; no behavior regressions apparent.src/utils/layout/all.test.tsx (1)
82-82: LGTM: Test wrapper aligns with new Subform override flow.Dropping the old TaskStore wrapper is consistent;
renderWithInstanceAndLayoutstill supplies required providers.src/features/instantiate/containers/InstantiationContainer.tsx (1)
21-28: LGTM: Removal of legacy provider.Direct rendering under
RenderStartis consistent with the refactor; no lost dependencies spotted.src/layout/Subform/index.tsx (1)
27-40: LGTM: Sub-routing now uses SubformWrapper with new overrides.Routes are intact; redirect preserved. Matches the TaskOverrides migration.
src/features/instance/useProcessTaskId.ts (1)
1-1: LGTM: Override source switched to TaskOverrides.Resolution order (override → process → URL) remains unchanged.
Also applies to: 6-9
src/features/form/layoutSets/useCurrentLayoutSet.ts (1)
2-2: LGTM: Layout set override sourced from TaskOverrides.Fallback to
getCurrentLayoutSetmaintained; nullability handling is correct.Also applies to: 16-26
src/test/renderWithProviders.tsx (2)
332-358: LGTM: Provider simplification reads clean and preserves loading orderLanguageProvider remains above SetShouldFetchAppLanguages and Router is wrapped with NavigationEffectProvider. Looks good.
383-388: LGTM: MinimalProviders keeps a lean treeAppropriate for lightweight tests; removing TaskStoreProvider here is safe.
src/features/attachments/StoreAttachmentsInNode.tsx (1)
5-5: LGTM: Switched to TaskOverridesImporting useTaskOverrides aligns with the refactor.
src/features/instantiate/selection/InstanceSelection.tsx (2)
46-54: LGTM: Provider removalWrapping with ActiveInstancesProvider and PresentationComponent is sufficient here.
237-290: LGTM: Wrapper fragmentNo behavioral change; title and ReadyForPrint remain intact.
src/layout/Summary2/CommonSummaryComponents/EditButton.tsx (1)
6-6: LGTM: Import moved to TaskOverridesMatches the new context.
src/features/datamodel/useBindingSchema.tsx (2)
37-49: LGTM: Overridden data element id respectedCorrectly short-circuits to the override in the selector.
133-148: LGTM: Overridden data model type respectedFalls back to computed type when no override is present.
src/features/datamodel/DataModelsProvider.tsx (1)
173-176: LGTM: Override plumbingUsing overrides to relax instance data element checks prevents false negatives during subform add flows.
src/index.tsx (1)
95-125: LGTM — confirm TaskStoreProvider/useTaskStore fully removedSandbox search produced no matches; run locally to verify no lingering imports/usages:
rg -n --hidden -S -uu '\b(TaskStoreProvider|useTaskStore)\b' --glob '!node_modules/**' || truesrc/features/form/layout/LayoutsContext.tsx (1)
86-103: LGTM — overrides are respected consistently.Both useLayoutSetId (LayoutsContext.tsx) and useCurrentLayoutSet/useCurrentLayoutSetId (useCurrentLayoutSet.ts) read useTaskOverrides(), and DataModelsProvider sets and compares its layoutSetId via useCurrentLayoutSetId() in BlockUntilLoaded — no change required.
src/layout/Summary2/SummaryComponent2/TaskSummaryWrapper.tsx (3)
1-3: LGTM! Clean import replacement.The import changes align with the refactor from TaskStore to TaskOverrides context.
17-17: LGTM! Direct data derivation improves clarity.The derivation of
layoutSetForTaskdirectly fromlayoutSetsandtaskIdis more straightforward than the previous approach using TaskStore state.
19-27: Verify TaskOverrides context consumption.useTaskOverrides()is invoked in:
- src/features/instance/useProcessTaskId.ts
- src/layout/Summary2/CommonSummaryComponents/EditButton.tsx
- src/layout/FileUpload/FileUploadTable/FileTableRow.tsx
- src/features/attachments/StoreAttachmentsInNode.tsx
- src/features/form/layoutSets/useCurrentLayoutSet.ts
- src/features/form/layout/LayoutsContext.tsx
- src/features/datamodel/useBindingSchema.tsx
- src/features/datamodel/DataModelsProvider.tsx
Confirm each consumer receives the correct
taskId,layoutSetId, anddataModelTypewhen rendered under this wrapper.src/layout/Subform/SubformWrapper.tsx (3)
6-6: LGTM! Proper context import.Adding TaskOverrides import aligns with the refactor to use the new context system.
17-22: Good delegation pattern.The refactor to delegate to
SubformOverrideWrappercreates a cleaner separation of concerns. The wrapper handles overrides while this component focuses on providing the FormProvider.
45-71: Approve — wrapper is correct; confirm desired failure modeSubformOverrideWrapper (src/layout/Subform/SubformWrapper.tsx) correctly derives actualDataElementId, resolves layoutSet/dataType and throws if dataType is missing.
- TaskOverrides is used to scope dataModelType/dataModelElementId/layoutSet.
- Error boundaries exist: src/components/ErrorBoundary.tsx (used in src/index.tsx), plus ComponentErrorBoundary (src/utils/layout/ComponentErrorBoundary.tsx) and GeneratorErrorBoundary (src/utils/layout/generator/GeneratorErrorBoundary.tsx).
If fail-fast is acceptable, no change needed. If you want graceful degradation, replace the throw with a user-facing fallback (Loader/error UI) or ensure callers always provide a valid layoutSet/dataType.
src/layout/Subform/Summary/SubformSummaryComponent2.tsx (4)
1-1: LGTM! Import changes support the refactor.Adding Fragment and TaskOverrides imports while removing TaskStoreProvider aligns with the context migration.
Also applies to: 7-7
58-58: Simplified keying strategy with Fragment.Using Fragment with
idxkey simplifies the wrapper structure. This is acceptable since the data elements array should remain stable within a render cycle.Also applies to: 67-67
105-140: Well-structured TaskOverrides integration.The TaskOverrides wrapper correctly provides:
dataModelElementIdfrom the data elementdataModelTypefrom the data elementlayoutSetIdfrom the layout setThe nested FormProvider with readOnly=true and the addition of LayoutSetSummary as a sibling component maintains the expected behavior while using the new context system.
94-96: Verify removal of TaskStore dependency.useSubformFormData now exposes isSubformDataFetching from useFormDataQuery (src/layout/Subform/utils.ts:22–34). Component early-returns while fetching:
if (isSubformDataFetching) { return null; }Confirm TaskOverrides (previously synced from TaskStore) are available before this component renders — repo search did not find useTaskOverrides (TSX search reported unrecognized file type); manual verification required to avoid timing/race issues.
…ed, and did much of the same stuff that useCurrentLayoutSetId() already did. The main difference was the taskId source, where useLayoutSetId() preferred the one from the URL, so I renamed it to reflect that.
|
github-project-automation
bot
moved this from 🔎 In review
to ✅ Done
in Team Altinn Studio
Description
Currently, taskStore is a context that is provided pretty much all around, and some places we set data in it (and then wait until that data is set in the zustand store before continuing rendering). This seemed overly complicated, as what we really need here (as far as i can tell) is just to provide some overrides to components below us - so a regular context will suffice and be easier to follow.
This PR cherry-picks such a refactor from #3731, where I really needed to provide multiple different sets of overrides side-by-side for rendering automatic PDFs for different tasks.
I also found a minor issue in the
useLayoutSetId()hook, which simply re-did much of whatuseCurrentLayoutSetId()already did. The only difference I could find was that it preferred the URL as a source oftaskIdover the current process data, which is whatuseCurrentLayoutSetId()preferred. Removed the cruft and renamed the hookuseLayoutSetIdFromUrl()to reflect what it actually is there to do.Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests