fix(widget): expose transaction signing errors#537
Conversation
|
📝 WalkthroughWalkthroughAdds an ChangesCustom external provider error propagation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/widget/src/pages/steps/hooks/use-steps.hook.ts (1)
171-183: RemoveuseMemoforcustomSignErrorMessageand compute it inline.The memoization at lines 171–183 wraps a derived primitive value (string or null) that is used only for its value in conditional rendering and text display. With React Compiler enabled, memoization for render-performance optimization of primitive values should be avoided; use plain computation instead.
Suggested change
- const customSignErrorMessage = useMemo(() => { - const error = machineState.context.currentTxMeta - .chainNullable((currentTxMeta) => { - return machineState.context.txStates[currentTxMeta.idx]?.meta.signError; - }) - .extractNullable(); - - if (!error || !("customMessage" in error)) return null; - - return typeof error.customMessage === "string" && error.customMessage - ? error.customMessage - : null; - }, [machineState.context.currentTxMeta, machineState.context.txStates]); + const error = machineState.context.currentTxMeta + .chainNullable((currentTxMeta) => { + return machineState.context.txStates[currentTxMeta.idx]?.meta.signError; + }) + .extractNullable(); + + const customSignErrorMessage = + error && + typeof error === "object" && + "customMessage" in error && + typeof error.customMessage === "string" && + error.customMessage + ? error.customMessage + : null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/steps/hooks/use-steps.hook.ts` around lines 171 - 183, The customSignErrorMessage variable is wrapped in a useMemo hook to memoize a derived primitive value (string or null), which is unnecessary with React Compiler enabled. Remove the useMemo wrapper entirely and compute the value inline by moving the logic inside useMemo directly into a const declaration, eliminating the useMemo call and its dependency array. This allows the primitive value to be computed on each render without unnecessary memoization overhead.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/widget/src/pages/steps/hooks/use-steps.hook.ts`:
- Around line 171-183: The customSignErrorMessage variable is wrapped in a
useMemo hook to memoize a derived primitive value (string or null), which is
unnecessary with React Compiler enabled. Remove the useMemo wrapper entirely and
compute the value inline by moving the logic inside useMemo directly into a
const declaration, eliminating the useMemo call and its dependency array. This
allows the primitive value to be computed on each render without unnecessary
memoization overhead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d89cb53c-32f0-41f5-821e-b5a8146919c4
📒 Files selected for processing (14)
packages/widget/src/domain/types/external-providers.tspackages/widget/src/pages/steps/hooks/errors.tspackages/widget/src/pages/steps/hooks/use-steps-machine.hook.tspackages/widget/src/pages/steps/hooks/use-steps.hook.tspackages/widget/src/pages/steps/pages/common.page.tsxpackages/widget/src/pages/steps/pages/styles.css.tspackages/widget/src/pages/steps/pages/tx-state.tsxpackages/widget/src/providers/sk-wallet/errors.tspackages/widget/src/providers/sk-wallet/index.tsxpackages/widget/src/styles/tokens/colors/contract.tspackages/widget/src/styles/tokens/colors/values.tspackages/widget/src/translation/English/translations.jsonpackages/widget/src/translation/French/translations.jsonpackages/widget/tests/use-cases/sk-wallet.test.tsx
| {txState.state === TxStateEnum.SIGN_ERROR ? ( | ||
| <Text variant={{ type: "danger" }}> | ||
| {txState.meta.signError?.message || t("steps.approve_error")} | ||
| {t("steps.approve_error")} |
There was a problem hiding this comment.
could this make non-custom signing errors less actionable for users? those cases now seem to collapse into the generic approve_error message
There was a problem hiding this comment.
those errors got moved in top section, while step label has generic error message
4 participants
Summary by CodeRabbit
New Features
Bug Fixes
Style