🔥 feat(web-sdk_angular): Add Angular Universal (SSR) support [NT-3467]#330
🔥 feat(web-sdk_angular): Add Angular Universal (SSR) support [NT-3467]#330Felipe Mamud (fmamud) wants to merge 8 commits into
Conversation
Felipe Mamud (fmamud)
requested a review
from David Nalchevanidze (nalchevanidze)
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
![wiz-inc-38d59fb8d7[bot]](https://avatars.githubusercontent.com/u/472182?s=60&v=4){kind=small}
Felipe Mamud (fmamud)
requested review from
Lotfi Anwar L Arif (Lotfi-Arif) and
Charles Hudson (phobetron)
as code owners
| * call). Lets call sites avoid an `if (context.platform === 'browser')` | ||
| * narrowing dance for fire-and-forget toggles. | ||
| */ | ||
| withSdk<T>(fn: (sdk: NgContentfulOptimizationInstance) => T): T | undefined { |
There was a problem hiding this comment.
I wouldn't say it's a blocker, but I was definitely confused by what withSdk was supposed to me until I read the comment. Maybe there's a name we could use that would better, but still concisely, describe its purpose?
There was a problem hiding this comment.
Good point, so renamed to ifBrowser across all call sites.
this.optimization.ifBrowser((sdk) => sdk.identify(...)).| * Runtime context for {@link NgContentfulOptimization}. Discriminated on | ||
| * `platform` so callers branch on the runtime instead of dereferencing a | ||
| * `sdk?` chain. The `server` branch carries no SDK because the Web SDK reads | ||
| * `localStorage` at construction time and cannot be instantiated server-side. |
There was a problem hiding this comment.
Maybe I'm missing some context, but we can use the Node SDK on the server side and pass its results to the client side, unless I'm missing something specific to this use case. I can see we're using the Node SDK later on in this file, so I guess I'm missing a nuance in this comment.
There was a problem hiding this comment.
I think you're right, the comment was misleading. Rewrote the docstring to make that split explicit, since the Node SDK does run server-side, and the service's context.sdk field only ever holds the browser Web SDK.
There was a problem hiding this comment.
This will be better when we provide @contentful/optimization-angular package, so that this will be similar to NestJS reference impl.
| // `provideServerOptimizationInitializer()` so `app.config.server.ts` only | ||
| // needs a single import to wire them in. | ||
|
|
||
| const APP_PERSONALIZATION_CONSENT_COOKIE = 'app-personalization-consent' |
There was a problem hiding this comment.
I'm wondering whether lower-level cookie handling can/should be moved out of the optimization service context, which I suppose could be similarly true for other reference implementations. It might make more sense regarding why this is not handled by the SDK, if we do not consider this cookie to be a managed part of the optimization process. The SDK can adjust its behavior on whether consent was granted, but the SDK can not determine how consent was gathered. Gathering consent, even granularly, is a consumer application responsibility.
There was a problem hiding this comment.
Agreed. I've xtracted app-level consent cookio into a new services/consent.ts and the browser-side ConsentCookie service writes via Angular's DOCUMENT injection token.
| /** | ||
| * Snapshot of the personalization context resolved server-side. Stamped into | ||
| * `TransferState` during SSR and read by browser code on hydration. | ||
| */ | ||
| export type ServerHandoff = | ||
| | { | ||
| readonly consent: false | ||
| } | ||
| | { | ||
| readonly consent: true | ||
| readonly profile: Profile | ||
| readonly profileId: string | ||
| readonly selectedOptimizations: SelectedOptimizationArray | ||
| } | ||
|
|
||
| /** | ||
| * Per-baseline-entry result of `sdk.resolveOptimizedEntry()` carried across | ||
| * the hydration boundary. The browser uses these to skip a duplicate Experience | ||
| * API roundtrip on initial render. Discriminated on `isVariant` so callers | ||
| * branch on the variant/baseline distinction without consulting a nullable | ||
| * field. | ||
| */ | ||
| export type ResolvedEntryHandoff = | ||
| | { | ||
| readonly isVariant: false | ||
| readonly baseline: Entry | ||
| readonly resolvedEntry: Entry | ||
| } | ||
| | { | ||
| readonly isVariant: true | ||
| readonly baseline: Entry | ||
| readonly resolvedEntry: Entry | ||
| readonly selectedOptimization: SelectedOptimization | ||
| } |
There was a problem hiding this comment.
I'm wondering whether this extra restructuring of SDK data might be overkill, or why we can't simply use the data as it's returned by the SDK.
There was a problem hiding this comment.
Fair, I agree. I've dropped the discriminated union and baseline field. Baselines moved to a separate SERVER_BASELINES_KEY so each slot stays semantically clean.
Felipe Mamud (fmamud)
force-pushed
the
NT-3467-angular-ssr
branch
from
670af1c to
187aefa
Compare
|
Charles Hudson (@phobetron) reviews has been addressed, could you check again pls? |
2 participants
Summary
implementations/web-sdk_angularwith Angular Universal SSR. Server-side SDK initialisation uses@contentful/optimization-nodevia a conditional dynamic import; the browserkeeps using
@contentful/optimization-webunchanged.TransferState. Anonymous-id continuity is preserved across runtimes via aSet-Cookiewritten by the server preflight.node-sdkreference:express@5.2.1+express-rate-limit@8.2.1, mounted on@angular/ssr/node'sAngularNodeAppEngine.What changed
src/main.server.ts,src/server.ts,src/app/app.config.server.ts,src/app/app.routes.server.ts.angular.jsongainsserver,outputMode: "server", andssr.entry.package.jsonadds@angular/ssr,@angular/platform-server,express,express-rate-limit, and astartscript.src/app/services/optimization-server.tsdynamic-imports@contentful/optimization-node, reads consent + anonymous-id cookies from Angular'sREQUESTtoken, calls
forRequest({ consent }).page(), runsresolveOptimizedEntry()per baseline, persists the anonymous-id back viaRESPONSE_INIT, and stampsTransferState.provideClientHydration(withEventReplay())on the browser side.src/app/services/entry.tsfalls back to TransferState when the browser SDK is absent (server runtime).src/app/services/contentful-client.tsloadEntriesshort-circuits when the server already fetched the baselines.NgContentfulOptimizationconstructs@contentful/optimization-webonly whenisPlatformBrowser. SDK call sites in components and services use null-safechains so server render skips browser-only side effects without a separate adapter type.
Why
Per the ticket implementation notes, "the existing browser SDK works cleanly in the SSR context without special-casing" was preferred — but the browser SDK touches
localStorageat construction, which crashes server-side. The fallback the notes explicitly allow is used:@contentful/optimization-nodefor server-side SDK initialisation, conditional dynamic imports for runtime separation. There are no unsafe type assertions: server modules use the Node SDK with its real types; browser modules use the Web SDK with its real types; onlyResolvedEntrydata crosses the boundary, viaTransferState.A future Angular adapter package will extract these helpers into
@contentful/optimization-angularand replace the in-impl wiring.Test plan
pnpm implementation:run -- web-sdk_angular typecheckpnpm exec eslint implementations/web-sdk_angularpnpm implementation:run -- web-sdk_angular build— emitsdist/web-sdk_angular/{browser,server}/pnpm implementation:run -- web-sdk_angular dev+ mocks):ng-server-context="ssr"cookie: app-personalization-consent=granted:data-ctfl-variant-index,data-ctfl-optimization-idpopulated server-side;Set-Cookie: ctfl-opt-aid=…returneddata-ctfl-baseline-idrendered; browser SDK takes over after hydration/page-tworoute also serves SSR'd HTMLResolving optimized entry for baseline entry …andhas been resolved to variant entry …lines per requesthttp://localhost:4200in a browser and exercise consent / identify / reset / live-updates / preview panel post-hydrationpnpm test:e2e:web-sdk_angular(existing scenarios were authored against the SPA; SSR-specific assertions are intentionally deferred)