-
Notifications
You must be signed in to change notification settings - Fork 51
🐛 Refactor generate-assets-wizard useWizardReducer to use useImmerReducer
#2658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🐛 Refactor generate-assets-wizard useWizardReducer to use useImmerReducer
#2658
Conversation
WalkthroughReplaces previous reducers with Immer-based draft reducers in two wizards, adds a lazy initial-state recipe and useImmerInitialState helper, computes isReady on the draft after mutations, changes RESET to carry a payload, and exposes stable setter callbacks and a reset function from the hook. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as App/Consumer
participant Hook as useWizardReducer(init?)
participant Init as useImmerInitialState
participant Reducer as wizardReducer (Immer draft)
participant Store as Hook State
UI->>Hook: initialize (optional init recipe)
Hook->>Init: run recipe once -> initial draft
Init-->>Hook: initial WizardState (ref-stored)
Hook-->>Store: set initial state
UI->>Hook: call setter (dispatch action)
Hook->>Reducer: dispatch(action) with draft
Reducer->>Reducer: mutate draft (fields, compute draft.isReady)
Reducer-->>Hook: mutated draft applied
Hook-->>Store: updated state visible to UI
rect rgba(0,128,0,0.06)
note right of Hook: RESET uses payload to replace draft
UI->>Hook: reset(payload)
Hook->>Reducer: dispatch({type: "RESET", payload})
Reducer-->>Hook: return payload as new draft (isReady recomputed)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (2)client/src/app/pages/applications/generate-assets-wizard/useWizardReducer.ts (5)
client/src/app/components/discover-import-wizard/useWizardReducer.ts (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). (2)
🔇 Additional comments (5)
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 |
c257b46 to
389a7c0
Compare
…Reducer` Following the pattern in discover-import-wizard from PR konveyor#2646, update generate-assets-wizard `useWizardReducer` to use immer and `useImmerReducer` for handling the initial state and all reducer actions. This helps keep the initial state clean and resettable, and helps keep the reducer logic simple and focused. Signed-off-by: Scott J Dickerson <[email protected]>
389a7c0 to
13fd9de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/src/app/pages/applications/generate-assets-wizard/useWizardReducer.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/app/pages/applications/generate-assets-wizard/useWizardReducer.ts (4)
client/src/app/api/models.ts (1)
TargetProfile(931-935)client/src/app/pages/applications/generate-assets-wizard/step-capture-parameters.tsx (1)
ParameterState(28-33)client/src/app/pages/applications/generate-assets-wizard/step-advanced-options.tsx (1)
AdvancedOptionsState(21-24)client/src/app/pages/applications/generate-assets-wizard/step-results.tsx (1)
ResultsData(26-36)
⏰ 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). (2)
- GitHub Check: unit-test
- GitHub Check: build-and-upload-for-global-ci
client/src/app/pages/applications/generate-assets-wizard/useWizardReducer.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Scott J Dickerson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/src/app/components/discover-import-wizard/useWizardReducer.ts(1 hunks)client/src/app/pages/applications/generate-assets-wizard/useWizardReducer.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
client/src/app/components/discover-import-wizard/useWizardReducer.ts (1)
client/src/app/pages/applications/generate-assets-wizard/useWizardReducer.ts (1)
WizardState(11-17)
client/src/app/pages/applications/generate-assets-wizard/useWizardReducer.ts (5)
client/src/app/components/discover-import-wizard/useWizardReducer.ts (3)
WizardState(10-15)InitialStateRecipe(62-62)useWizardReducer(78-118)client/src/app/api/models.ts (1)
TargetProfile(931-935)client/src/app/pages/applications/generate-assets-wizard/step-capture-parameters.tsx (1)
ParameterState(28-33)client/src/app/pages/applications/generate-assets-wizard/step-advanced-options.tsx (1)
AdvancedOptionsState(21-24)client/src/app/pages/applications/generate-assets-wizard/step-results.tsx (1)
ResultsData(26-36)
⏰ 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). (2)
- GitHub Check: build-and-upload-for-global-ci
- GitHub Check: unit-test
🔇 Additional comments (7)
client/src/app/components/discover-import-wizard/useWizardReducer.ts (1)
38-41: LGTM!The explicit type annotation for the reducer signature is clearer and correctly encodes Immer's contract where reducers can either mutate the draft (void) or return a replacement state (WizardState).
client/src/app/pages/applications/generate-assets-wizard/useWizardReducer.ts (6)
1-3: LGTM!The imports are appropriate for the Immer-based refactoring and align with the established pattern in the discover-import-wizard.
38-38: LGTM!The RESET action with a WizardState payload enables proper reset functionality and follows the established pattern.
48-73: Reducer logic is sound.The reducer correctly implements Immer's draft-mutation pattern for SET_* actions and properly updates
isReadyafter each mutation. The explicit return typeWizardState | voidcorrectly documents the contract.Note: The RESET case issue is addressed in a separate comment.
75-75: LGTM!The
InitialStateRecipetype provides a clean API for customizing initial state and matches the established pattern.
77-89: LGTM!The lazy initialization pattern correctly computes the initial state once, applies the optional recipe, and ensures
isReadyis computed via the reducer. The use ofuseRefprevents recomputation on re-renders as noted in the linked React documentation.
91-140: LGTM!The hook implementation follows React best practices with stable callbacks via
useCallback, proper dependency arrays, and lazy initialization. The API is clean and type-safe, matching the established pattern in discover-import-wizard.
client/src/app/pages/applications/generate-assets-wizard/useWizardReducer.ts
Outdated
Show resolved
Hide resolved
… avoid immer issues Signed-off-by: Scott J Dickerson <[email protected]>
…Reducer` (#2658) Following the pattern in discover-import-wizard from PR #2646, update generate-assets-wizard `useWizardReducer` to use immer and `useImmerReducer` for handling the initial state and all reducer actions. This helps keep the initial state clean and resettable, and helps keep the reducer logic simple and focused. No visible changes. ## Summary by CodeRabbit - **Refactor** - Reworked wizard state management to use an immutable-draft approach for more consistent initialization, stable setter references, and reliable reset behavior. - **Bug Fixes** - Fixed readiness computation so Continue/Next reflects profile, parameters, filters, and advanced options immediately and consistently. --------- Signed-off-by: Scott J Dickerson <[email protected]> Signed-off-by: Cherry Picker <[email protected]>
…Reducer` (#2658) (#2662) Following the pattern in discover-import-wizard from PR #2646, update generate-assets-wizard `useWizardReducer` to use immer and `useImmerReducer` for handling the initial state and all reducer actions. This helps keep the initial state clean and resettable, and helps keep the reducer logic simple and focused. No visible changes. ## Summary by CodeRabbit - **Refactor** - Reworked wizard state management to use an immutable-draft approach for more consistent initialization, stable setter references, and reliable reset behavior. - **Bug Fixes** - Fixed readiness computation so Continue/Next reflects profile, parameters, filters, and advanced options immediately and consistently. --------- Signed-off-by: Scott J Dickerson <[email protected]> Signed-off-by: Cherry Picker <[email protected]> Co-authored-by: Scott Dickerson <[email protected]>
…Reducer` (konveyor#2658) Following the pattern in discover-import-wizard from PR konveyor#2646, update generate-assets-wizard `useWizardReducer` to use immer and `useImmerReducer` for handling the initial state and all reducer actions. This helps keep the initial state clean and resettable, and helps keep the reducer logic simple and focused. No visible changes. ## Summary by CodeRabbit - **Refactor** - Reworked wizard state management to use an immutable-draft approach for more consistent initialization, stable setter references, and reliable reset behavior. - **Bug Fixes** - Fixed readiness computation so Continue/Next reflects profile, parameters, filters, and advanced options immediately and consistently. --------- Signed-off-by: Scott J Dickerson <[email protected]>
Following the pattern in discover-import-wizard from PR #2646, update generate-assets-wizard
useWizardReducerto use immer anduseImmerReducerfor handling the initial state and all reducer actions. This helps keep the initial state clean and resettable, and helps keep the reducer logic simple and focused.No visible changes.
Summary by CodeRabbit
Refactor
Bug Fixes