[ENG-10194] Angular SDK – reset builder-context when content id changes #4162
Conversation
🦋 Changeset detectedLatest commit: 46f284c 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
View your CI Pipeline Execution ↗ for commit 46f284c
☁️ Nx Cloud last updated this comment at |
samijaber
left a comment
samijaber
left a comment
There was a problem hiding this comment.
Could we add some e2e tests for this client-side navigation + repeater issue to make sure we know which SDKs need the fix?
| contentId: nextId, | ||
| }); | ||
| } | ||
| }, [props.content, props.data, props.locale]); |
There was a problem hiding this comment.
Is this only needed for angular SDK? If so, lets wrap this new logic in useTarget to make sure it only runs for angular. No need to redundantly update state for other SDKs if they dont need it
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 22600423 | Triggered | Generic High Entropy Secret | e38b073 | packages/sdks/e2e/angular-20-ssr/src/app/catch-all-page/catch-all-page.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
packages/sdks-tests/src/e2e-tests/ssr-navigation-repeat.spec.ts
Outdated
Show resolved
Hide resolved
| }, | ||
| default: () => {}, | ||
| }); | ||
| }, [props.content, props.data, props.locale]); |
There was a problem hiding this comment.
Bug: jsCode not re-executed on content change
When content changes in Angular (detected by ID change), the onUpdate hook updates builderContextSignal.value.content and rootState but doesn't re-execute the new content's jsCode. The jsCode evaluation only happens in onInit (line 180), so when navigating between pages with different content, the new page's jsCode never runs. This causes the new content's state initialization and side effects to be skipped, potentially breaking functionality that depends on jsCode execution.
| updateContentAndRootState(); | ||
| }, | ||
| default: () => {}, | ||
| }); |
There was a problem hiding this comment.
Bug: useTarget wrapped in onUpdate not executing
The useTarget call returns a function but never invokes it. The updateContentAndRootState() function will never execute on any target. The return value from useTarget needs to be called to actually execute the Angular-specific logic for resetting builder context when content ID changes.
| state.runHttpRequests(); | ||
| }); | ||
| } | ||
| state.mergeNewContent(props.content); |
There was a problem hiding this comment.
Bug: Redundant content merge after parent replacement
For Angular, the parent component completely replaces builderContextSignal.value.content when the content ID changes, but the child component unconditionally calls state.mergeNewContent(props.content) on every content update. This creates redundant merging when IDs change and potential race conditions since both components modify the same signal. The child's merge at line 347 should be conditional on the ID not changing, similar to the parent's logic, to avoid conflicts.
Additional Locations (1)
The issue is specific to Angular Unfortunately, I could not add the e2e test because it depends on |
4 participants
Description
• Detect
@Input() contentswitch (id differs from current)• Re-initialise
builderContextSignal.content&rootState• Regenerate AB-test visibility script
This ensures repeaters/blocks re-render correctly after SSR refresh and route reuse, eliminating the need for custom
RouteReuseStrategyhacks.blueprints-editorial.spec.tssnippet test for ssr frameworks since thefakestoreapimocked in this PR doesn't not work for ssr frameworks leading to them being flaky, it works only for csr.JIRA
https://builder-io.atlassian.net/browse/ENG-10194
Loom
https://www.loom.com/share/47d4f285da2a45f2bd1ff6cc82bdad4c?sid=16e0dad3-3104-4fbf-99fd-7f4b1378695a
Note
Fixes Angular SSR hydration when navigating by resetting content/root state and scripts on content ID change, and updates tests/skip lists; adds patch changeset.
content.lite.tsx: On Angular target, detectprops.content.idchanges and reinitializebuilderContextSignal.contentandrootState; regenerate AB-test visibility script.enable-editor.lite.tsx: On Angular target, when content changes, rerun HTTP requests on ID change and merge new content during update.blueprints-editorial.spec.ts: Expand skipped packages to cover additional SSR frameworks.custom-components.spec.ts: Broaden Angular SSR check/message to include Angular 17 and 19.@builder.io/sdk-angularaddressing SSR hydration during routing.Written by Cursor Bugbot for commit 46f284c. This will update automatically on new commits. Configure here.