Fix CustomCode scriptsClientOnly not re-running scripts after hydration - #4848
Manish-Builder-io wants to merge 6 commits into
Conversation
|
View your CI Pipeline Execution ↗ for commit 80f6050
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
|
@builderio-bot Review all unresolved PR comments and latest review feedback. Be skeptical and verify each suggestion against the code and requirements before making changes. For every comment:
Process every comment and finish with a summary of fixed, rejected, and blocked items. |
|
All checks pass. Summary of this round: Fixed (1)
Replied on the thread and resolved it. Found while verifying (not raised by any reviewer) CI runs Rejected: none. Blocked: none. Verification: exact CI command passes; full package suite 48 passed; One thing I left alone deliberately: the merge from |
🦋 Changeset detectedLatest commit: 13f418b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -0,0 +1,5 @@ | |||
| --- | |||
There was a problem hiding this comment.
Will do a dev release first
|
@builderio-bot Re-investigate the Custom Code |
|
Re-investigated from scratch as requested, without assuming the original diagnosis. The reviewer's hypothesis is confirmed — Confirmed root cause
With
Why Why the previous test passed while production failed: the test inserted the SSR markup into FixOne targeted change, in get isHydrating() {
return !isShopify && Boolean(this.originalRef || this.props.scriptsClientOnly);
}
The Regression testAdded Both fixes proven load-bearing by reverting each independently:
Behavior preserved
Validation
|
|
The reviewer was right, and the original diagnosis was incomplete. Confirmed root cause: React does not repair The old test inserted the SSR markup before requiring the module, so Fix — one line in return !isShopify && Boolean(this.originalRef || this.props.scriptsClientOnly);The Added One thing to flag: the working tree also contained unrelated version downgrades in |
There was a problem hiding this comment.
Builder reviewed your changes — looks good ✅
Review Details
Incremental Code Review Summary
The latest update expands the CustomCode fix to cover asynchronous SDK/chunk loading, where the module-level SSR node scan can occur before markup is present. isHydrating now also treats scriptsClientOnly as sufficient to keep the initial client render script-free, allowing the hydration-state transition to restore and execute scripts afterward. The added regression test explicitly exercises module-before-markup loading, alongside the existing SSR, hydration, and render-bailout coverage.
Two parallel reviewers found no new confirmed issues. The focused four-test suite passes, the package build succeeds, and the version/changeset updates remain consistent with the repository's prerelease flow. Existing unrelated full-suite module-resolution failures were not attributed to this PR.
Risk remains standard.
🧪 Browser testing: Skipped — dependency setup is still running and no development server or proxy is available; will retry on the next incremental review.
| // module was evaluated. Bundlers load the SDK chunk asynchronously, so that ordering is not | ||
| // guaranteed. `scriptsClientOnly` always strips scripts on the server, so the first client | ||
| // render has to match that regardless of whether the node was captured. | ||
| return !isShopify && Boolean(this.originalRef || this.props.scriptsClientOnly); |
There was a problem hiding this comment.
This change kinda makes sense. I will do another dev release for this one tomorrow
There was a problem hiding this comment.
@sanyamkamat Could you please do a dev release today for testing.
Description
Fixes a regression where
CustomCodeblocks withscriptsClientOnly: truefail to restore SSR-stripped<script>tags after hydration.Root cause:
shouldComponentUpdateinCustomCodeComponentonly comparednextProps.code !== this.props.code. This meant thesetState({ hydrated: true })call made incomponentDidMount(which is meant to trigger a one-time re-render to re-insert the scripts that were stripped for SSR) was blocked, since onlystate.hydratedchanged and notprops.code.Fix:
Statetype ({ hydrated: boolean }) and used it as the generic parameter onReact.Component<Props, State>instead of leaving the component untyped for state.shouldComponentUpdateto also acceptnextState: Stateand returntruewhennextState.hydrated !== this.state.hydrated, in addition to the existingcodeprop comparison. This preserves the original perf optimization (skipping re-renders whencodeis unchanged) while no longer suppressing the one-time hydration re-render.Tests:
packages/react/test/custom-code.test.tsxcovering:scriptsClientOnlyis true.CustomCodestill skips unnecessary re-renders when neithercodenor hydration state change (verifying no perf regression).packages/react/test/setupTests.tsto polyfillTextEncoder/TextDecoderin the jsdom test environment, whichreact-dom/server'srenderToStringrequires but jsdom 19 doesn't provide.@builder.io/react.Link to JIRA ticket:
https://builder-io.atlassian.net/browse/SUPP-2847
To clone this PR locally use the Github CLI with command
gh pr checkout 4848You can tag me at @BuilderIO for anything you want me to fix or change
Note
Low Risk
Narrow lifecycle fix in CustomCode with regression tests; behavior change is restoring intended client-only script execution after hydration.
Overview
Fixes a regression where Custom Code blocks with
scriptsClientOnlystripped<script>tags on SSR but never put them back or executed them on the client.shouldComponentUpdateonly comparedprops.code, so the one-timesetState({ hydrated: true })incomponentDidMountdid not trigger a re-render. The change types component state as{ hydrated: boolean }and allows updates whenhydratedchanges, while still skipping re-renders when only unrelated parent props change.Adds jsdom tests for SSR script stripping, post-hydration script restore/execution, and a render-count guard. Test setup polyfills
TextEncoder/TextDecoderforrenderToString. Patch changeset for@builder.io/react.Reviewed by Cursor Bugbot for commit 274496e. Bugbot is set up for automated code reviews on this repo. Configure here.