SCAL-313306: refactor declarative embedParams building in V1Embed#547
SCAL-313306: refactor declarative embedParams building in V1Embed#547yinstardev wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized, declarative embedParams builder system (embedParams-builder.ts) to aggregate configuration contributions across various embed classes (AppEmbed, SpotterEmbed, LiveboardEmbed, and SearchEmbed), replacing the previous imperative getAppInitData overrides. The review feedback recommends updating buildEmbedParams to return undefined instead of an empty object when no contributions exist, which avoids bloating the payload. Following this change, the subclass implementations of getEmbedParamsContributions can be simplified to be fully declarative, eliminating redundant boilerplate checks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| protected async getEmbedParamsContributions(): Promise<Partial<EmbedParamsContribution>[]> { | ||
| const contributions: Partial<EmbedParamsContribution>[] = []; | ||
|
|
||
| const sidebarContribution = buildSpotterSidebarEmbedParamsContribution( | ||
| this.viewConfig, | ||
| this.handleError.bind(this), | ||
| ); | ||
| if (Object.keys(sidebarContribution).length > 0) { | ||
| contributions.push(sidebarContribution); | ||
| } | ||
|
|
||
| return contributions; | ||
| } |
There was a problem hiding this comment.
With buildEmbedParams updated to safely filter out empty contributions and return undefined when empty, we can simplify this method to be completely declarative and eliminate the imperative boilerplate of checking Object.keys().length and pushing to an array.
protected async getEmbedParamsContributions(): Promise<Partial<EmbedParamsContribution>[]> {
return [
buildSpotterSidebarEmbedParamsContribution(
this.viewConfig,
this.handleError.bind(this),
),
];
}
commit: |
a1f7dc1 to
8e0ac2b
Compare
Replace the imperative, spread-chain embedParams construction with a single declarative mapping, removing the per-subclass overrides and the mutation chains they relied on. Before: each embed (App/Liveboard/Search/Spotter) overrode getAppInitData and hand-wired builders that took the whole payload, reached into the previous builder's embedParams via `as any`, and re-spread it. This was order-dependent, duplicated across subclasses, and handled visualOverrides in two separate paths. After: - New embedParams-builder.ts owns the single viewConfig -> embedParams mapping as a declarative table (EMBED_PARAM_FIELDS). Trivial fields are pass-throughs; spotterSidebarConfig (legacy-flag merge + URL validation) is the one resolver with real logic. buildEmbedParamsPayload() runs the table and merges results. - TsEmbed.getDefaultAppInitData() calls buildEmbedParamsPayload() once, so every embed type gets embedParams with zero per-subclass wiring. - Deleted getAppInitData overrides from AppEmbed, LiveboardEmbed, SpotterEmbed; SearchEmbed keeps only its top-level searchOptions merge. - visualOverrides is now one row in the table instead of two divergent paths. Adding a new embedParams field is now a one-row change in embedParams-builder.ts. No change to the emitted embedParams shape. tsc + eslint clean; full SDK suite passes (1103 passed, 4 skipped).
8e0ac2b to
a5f203f
Compare
Replace imperative embedParams construction with declarative pattern to eliminate mutation chains and improve maintainability. Each embed type now declares contributions separately, merged once in base class.
Changes:
Benefits:
Tests: all 1103 tests pass