setting current value, to correctly handle change #57
Conversation
✅ Deploy Preview for fluffy-codec ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughA mutable ref is added to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (2)
src/pages/Codec.tsx (2)
55-75: Consider decodingnewInputinstead ofbytesInputto avoid stale stateInside
handleBytesToJson, you ignore thenewInputargument when decoding and instead use thebytesInputstate. Because state updates are async, this can leave the decode step one render behind (especially ononChangepaths or URL-driven updates) and makes the logic more sensitive to closure timing than necessary.You can make the function more direct and robust by decoding the value you were just given:
- const decoded = codec.Decoder.decodeObject<unknown>( - kindDescriptor.clazz.Codec, - bytes.BytesBlob.parseBlob(bytesInput), - spec?.spec, - ); + const decoded = codec.Decoder.decodeObject<unknown>( + kindDescriptor.clazz.Codec, + bytes.BytesBlob.parseBlob(newInput), + spec?.spec, + );This aligns the decode with the source of truth for the current change and avoids relying on when
setBytesInputhas taken effect.
102-115: Ref update fixes stale callback, but effect can still see previouskind/chainSpecfor URL-only changesUpdating
handleBytesToJsonRef.currenton every render is the right move to avoid the “ref holds the initial callback” problem; the effect will now always call the latesthandleBytesToJson.However, the
useEffectbelow still does:if (validSearchParams.kind !== undefined) { setKind(validSearchParams.kind.name); } if (validSearchParams.chainSpec !== undefined) { setChainSpec(validSearchParams.chainSpec.name); } if (validSearchParams.data !== null) { handleBytesToJsonRef.current(validSearchParams.data); }Because
setKind/setChainSpecare async, the call tohandleBytesToJsonRef.current(...)runs with whateverkind/chainSpecwere in the render that scheduled this effect. If the URL query can change from outside this component (e.g., navigation that only updateskind/flavor), you can still end up decoding and even rewriting the URL using the previouskind/chainSpecinstead of what’s invalidSearchParams.To make this more robust, consider one of these approaches:
- Let
handleBytesToJsonaccept optional overrides and use them in the effect:// signature sketch const handleBytesToJson = ( newInput: string, opts?: { kindOverride?: string; chainSpecOverride?: string }, ) => { const effectiveKind = opts?.kindOverride ?? kind; const effectiveChainSpec = opts?.chainSpecOverride ?? chainSpec; // use effectiveKind / effectiveChainSpec everywhere }; // in the effect if (validSearchParams.data !== null) { handleBytesToJsonRef.current(validSearchParams.data, { kindOverride: validSearchParams.kind?.name, chainSpecOverride: validSearchParams.chainSpec?.name, }); }
- Or, move the decode logic used for URL-driven initialisation into the effect itself and base it purely on
validSearchParamsrather than the component state.Also, minor/optional: if you’d like to avoid mutating the ref during render, you can instead update it in a small
useEffectthat depends onhandleBytesToJson; the current approach is common and works, but an effect keeps all writes out of render if you prefer that style.
useRef(handleBytesToJson)captures thehandleBytesToJsonfunction at the time of the first render.When
kindorchainSpecstate updates, the function stored inhandleBytesToJsonRef.currentstill has the old values in its closure.Addin
handleBytesToJsonRef.current = handleBytesToJson;updates the ref to point to the current version ofhandleBytesToJsonon every render, which has the current values ofkindandchainSpecin its closure.