Decouple the prebid tsjs shim from the bundled Prebid.js - #967
Conversation
The external bundle is now pure Prebid.js (core, consent and user ID modules, client-side bid adapters) and stamps a manifest on window.__tsjs_prebid_bundle. The shim ships as a server-served deferred tsjs module that installs the trustedServer adapter onto the window.pbjs global via public APIs only, so shim fixes deploy with the server instead of requiring an external bundle re-upload.
Widen the lint script to cover test/**, and make every test file pass it with real types: typed window views, TestBid/TestAdUnit shapes, adapter spec and requestBids parameter types, typeof-fetch casts for fetch mocks, and signature-free spies instead of unused typed parameters.
# Conflicts: # crates/trusted-server-js/lib/src/integrations/prebid/index.ts # crates/trusted-server-js/lib/test/integrations/prebid/index.test.ts
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Splitting the shim out of the external Prebid bundle is the right call, and the mechanics hold up: the head injector emits the window.pbjs stub and the external bundle <script defer> via head_inserts, then html_processor appends the deferred tsjs tags, so the bundle always precedes tsjs-prebid in document order and both are plain defer (neither is async). Replacing the internal adapterManager.getBidAdapter import with a manifest stamped by the bundle is a genuine improvement to the public-API boundary.
The concerns below are about the new seam between the two artifacts: they can now fail independently, and one of those partial-load states actively degrades the page.
Blocking
🔧 wrench
- Bail-out is partial — the refresh handler still installs when the external bundle is missing:
installPrebidNpm()returns early, but self-init callsinstallRefreshHandler()unconditionally, so publisher GPT refreshes clearts_initial/hb_*targeting (including server-side SSAT targeting applied byadInit) and then fail to run any auction. New failure mode — onmaina failed bundle meant no shim at all. (crates/trusted-server-js/lib/src/integrations/prebid/index.ts:834-845,:1353-1356) - The new bail-out branch has no test: the test harness always installs a full
mockPbjs, so the PR's central new failure path — external bundle did not load — is uncovered. (crates/trusted-server-js/lib/test/integrations/prebid/index.test.ts:112-133)
Non-blocking
🤔 thinking
processQueue()ownership moved into the second artifact: bundle loads but shim does not (adblock ontsjs-prebid.min.js,/static/tsjs=error, CSP) →pbjs.quenever drains andwindow.pbjsstill looks loaded. Consider a watchdogprocessQueue()in the generated bundle entry. (index.ts:1070,build-prebid-external.mjs:197)- No idempotency guard for the rollout window: a
window.__tsjsPrebidShimInstalledsentinel would make new-server/old-bundle safe in either deploy order and remove the ordering constraint from the runbook. (index.ts:834) - The manifest is page-controlled state read without shape validation: a non-array
adapters/userIdModulesmakes.includes()throw out ofinstallPrebidNpmat module scope. (index.ts:57-62,:1078-1095)
♻️ refactor
- Injection order is an untested correctness invariant: the test asserts both script tags are present, not that the bundle tag precedes
tsjs-prebid.min.js. Reversed order hard-fails the integration. (crates/trusted-server-core/src/integrations/prebid.rs:3018-3026) - User ID diagnostics do not distinguish "no manifest" from "module missing": unlike the adapters check, this path falls back to
[]and warns once per configured module against an unstamped bundle. (index.ts:171-213)
🌱 seedling / 🏕 camp site
- Dead placeholders:
src/integrations/prebid/_adapters.generated.tsand_user_ids.generated.tsare no longer imported by anything undersrc/— the external builder writes its own copies into the temp dir and aliases those specifiers. Safe to delete. - No guard keeping the shim Prebid-free: a future value-import of
'prebid.js'in place ofimport typewould silently re-bundle Prebid core into tsjs and double-install it. A size or marker assertion ondist/tsjs-prebid.jswould catch that regression, since the "no Prebid core markers" check is currently manual.
📝 note
- Docs omit the lockstep rollout: the "External Bundle Generation" section of
docs/guide/integrations/prebid.mddoes not state that the bundle is now pure Prebid.js, nor that an already-deployed bundle must be regenerated together with this server change. That constraint currently lives only in the PR description, where operators will not find it later.
⛏ nitpick
- Adapter-missing error names the wrong operator surface: it points at
build-prebid-external.mjs --adaptersrather thants prebid bundle/[integrations.prebid.bundle].adapters. (index.ts:1090)
👍 praise
- Type-only import plus window-global capture is the right seam, and dropping
adapterManager.getBidAdapterfor the stamped manifest removes the last dependency on Prebid internals with no loss of validation. (index.ts:14,:1073-1095) - Test-file cleanup: replacing
anycasts with typed test views and extendinglinttotest/**is enforced by CI (format.yml:87,:118), so it will not rot.
CI Status
- fmt: PASS
- clippy: PASS (fastly, axum, cloudflare native + wasm, spin native + wasm)
- rust tests: PASS (fastly, axum, cloudflare, spin, cross-adapter parity, ts CLI)
- js tests: PASS (vitest, browser + integration suites)
| '[tsjs-prebid] window.pbjs has no Prebid.js API — the external Prebid bundle ' + | ||
| 'failed to load. Prebid integration disabled.' | ||
| ); | ||
| return pbjs; |
There was a problem hiding this comment.
🔧 wrench — Bail-out is partial: the refresh handler still installs when the external bundle is missing.
installPrebidNpm() returns early here, but self-init calls installRefreshHandler() unconditionally right after it (line 1355). That wrapper is then live on googletag.pubads().refresh(), and on every publisher refresh it runs clearRefreshTargeting() on each independent slot — clearing ts_initial, hb_pb, hb_bidder, hb_adid, hb_cache_host, hb_cache_path — before calling pbjs.requestBids(...), which is undefined on the head-injected stub. The TypeError is caught by the try/catch at line 1243 and completeRefresh(false) skips setTargetingForGPTAsync, so nothing refills the targeting.
Net effect when the bundle fails to load (network error, SRI mismatch, blocked request): server-side SSAT targeting applied by adInit is wiped on the first publisher refresh and no auction replaces it.
This failure mode is new to this PR. On main the shim lived inside the external bundle, so a failed bundle meant no shim at all and nothing wrapped pubads.refresh. Splitting the artifacts makes "shim present, Prebid absent" reachable.
Fix — gate the rest of self-init on a successful install:
const prebidApiAvailable =
typeof (pbjs as { registerBidAdapter?: unknown }).registerBidAdapter === 'function';
if (typeof window !== 'undefined') {
installPrebidNpm();
if (prebidApiAvailable) {
installRefreshHandler();
// ...user ID module setup
}
}| __tsjs_prebid_bundle?: unknown; | ||
| }; | ||
| w.pbjs = mockPbjs; | ||
| w.__tsjs_prebid_bundle = { |
There was a problem hiding this comment.
🔧 wrench — The new bail-out branch has no test.
The vi.hoisted() block always installs a full mockPbjs (with registerBidAdapter) and a stamped __tsjs_prebid_bundle, so no test exercises the PR's central new failure path: the external Prebid bundle did not load. Grepping this file for registerBidAdapter deletion, Prebid integration disabled, or has no Prebid.js API returns nothing.
Add a case that installs a bare { que: [], cmd: [] } global (module registry reset so the shim re-captures it) and asserts:
- the error is logged,
pbjs.requestBidsis left unwrapped,- after the fix for the refresh-handler finding,
googletag.pubads().refreshis not wrapped and publisher targeting survives a refresh.
| for (const bidder of clientSideBidders) { | ||
| try { | ||
| if (!adapterManager.getBidAdapter(bidder)) { | ||
| // Validate that every client-side bidder has its adapter compiled into the |
There was a problem hiding this comment.
🤔 thinking — processQueue() ownership now lives in the second artifact.
The external bundle deliberately does not call processQueue(); the shim does (line 1070). That makes the reverse partial-load a silent failure: if the bundle loads but the shim does not (adblock filters matching tsjs-prebid.min.js, a /static/tsjs= error, CSP), pbjs.que never drains. Publisher code that pushed into pbjs.que never runs, while window.pbjs looks fully loaded, so there is no signal anywhere on the page.
Before this PR a single artifact made this atomic. Worth considering a watchdog in the generated bundle entry that calls processQueue() itself if the shim has not installed within N ms, so the queue drains in a degraded but functional state.
| * 2. `config` argument — explicit overrides from the publisher's JS | ||
| */ | ||
| export function installPrebidNpm(config?: Partial<PrebidNpmConfig>): typeof pbjs { | ||
| // The prebid integration requires the external Prebid.js bundle |
There was a problem hiding this comment.
🤔 thinking — No idempotency guard for the rollout window.
The deployment note says the currently deployed bundle still carries the old baked-in shim, so running the new server against it installs the adapter twice and wraps requestBids twice. A sentinel here makes either deploy order safe for about three lines, and __tsRemoveAdUnitWrapped already sets the precedent in this file:
const installedFlag = window as { __tsjsPrebidShimInstalled?: boolean };
if (installedFlag.__tsjsPrebidShimInstalled) return pbjs;
installedFlag.__tsjsPrebidShimInstalled = true;That removes the ordering constraint from the deploy runbook entirely.
| assert!( | ||
| !processed.contains("tsjs-prebid.min.js"), | ||
| "Embedded deferred prebid bundle should not be injected" | ||
| processed.contains("tsjs-prebid.min.js"), |
There was a problem hiding this comment.
♻️ refactor — Injection order is now a correctness invariant, and it is untested.
Both scripts are defer, so they execute in document order: the external bundle tag (emitted by head_inserts) must precede the tsjs-prebid deferred tag (emitted afterwards by html_processor). If that order ever reverses, the shim runs first, finds no registerBidAdapter, and disables the integration outright — a hard failure with no test to catch it.
This test asserts both tags are present but not their relative position. Suggest tightening it:
let bundle_index = processed
.find(PREBID_BUNDLE_ROUTE)
.expect("should inject external prebid bundle route");
let shim_index = processed
.find("tsjs-prebid.min.js")
.expect("should inject deferred tsjs prebid shim");
assert!(
bundle_index < shim_index,
"external prebid bundle must execute before the deferred tsjs shim"
);| } | ||
|
|
||
| function recordUserIdModuleDiagnostics(): PrebidUserIdDiagnostics { | ||
| const includedUserIdModules = getExternalBundleManifest()?.userIdModules ?? []; |
There was a problem hiding this comment.
♻️ refactor — User ID diagnostics do not distinguish "no manifest" from "module missing".
The adapters check at line 1078 explicitly handles manifest === undefined with a single "did not stamp an adapter manifest" warning. This path instead falls back to [], so an older or unstamped bundle makes every configured User ID module look absent and emits one warning per module on the first auction.
Mirroring the adapters handling keeps the two diagnostics consistent and avoids misleading operators during the bundle rollout.
| if (typeof window === 'undefined') { | ||
| return undefined; | ||
| } | ||
| return (window as { __tsjs_prebid_bundle?: ExternalPrebidBundleManifest }).__tsjs_prebid_bundle; |
There was a problem hiding this comment.
🤔 thinking — The manifest is page-controlled state read without shape validation.
__tsjs_prebid_bundle is a plain window global that any script on the page can set. The interface types adapters and userIdModules as string[], but nothing checks at runtime: if either is present and non-array, bundledAdapters.includes(bidder) (line 1088) or includedUserIdModules.includes(entry.moduleName) (line 176) throws a TypeError, which escapes installPrebidNpm and, on the self-init path, becomes an uncaught error at module scope.
A pair of Array.isArray guards inside getExternalBundleManifest keeps a malformed or hostile global to a diagnostics-only degradation.
| log.error( | ||
| `[tsjs-prebid] client-side bidder "${bidder}" has no adapter loaded. ` + | ||
| `Add it to build-prebid-external.mjs --adapters.` | ||
| `[tsjs-prebid] client-side bidder "${bidder}" has no adapter in the external ` + |
There was a problem hiding this comment.
⛏ nitpick — This message names the wrong operator surface. The documented flow is ts prebid bundle driven by [integrations.prebid.bundle].adapters in trusted-server.toml; build-prebid-external.mjs --adapters is the internal script the CLI shells out to. Pointing at the CLI config key matches what an operator can actually change.
| // shim leaves its bids untouched and the corresponding adapter handles them | ||
| // natively in the browser. | ||
| import './_adapters.generated'; | ||
| import type _pbjsDefault from 'prebid.js'; |
There was a problem hiding this comment.
👍 praise — This is the right seam. A type-only import plus capturing the head-injected window.pbjs stub keeps Prebid.js entirely out of the tsjs artifact while preserving the typing, and replacing the internal adapterManager.getBidAdapter import with the stamped manifest drops the last dependency on Prebid internals. Validation quality is unchanged, the coupling is gone.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Review Summary
The artifact split is directionally sound: the external Prebid bundle is injected before the deferred shim, and the shim uses the public global API boundary. I found one high-priority validation regression and one important test gap; both are inline.
Please address the adapter-code validation before relying on the new production diagnostic.
| '// Manifest consumed by the tsjs prebid shim to validate that every', | ||
| '// configured client_side_bidder has its adapter compiled in.', | ||
| '(window as unknown as Record<string, unknown>).__tsjs_prebid_bundle = Object.freeze({', | ||
| ` adapters: ${JSON.stringify(adapters)},`, |
There was a problem hiding this comment.
P1 — Adapter manifest records module filenames, not registered bidder codes.
adapters contains the requested file stem, but client_side_bidders contains the runtime Prebid bidder code. They are not equivalent in Prebid 10.26.0: a1MediaBidAdapter.js registers a1media, and adfBidAdapter.js registers adf plus adform / adformOpenRTB aliases. A valid alias therefore logs a false missing-adapter error, while a filename stem can pass this check despite no adapter being registered for that bidder code.
Please stamp a separate list of registered bidder codes (including aliases), derived from metadata/modules/<adapter>BidAdapter.json, and validate client_side_bidders against that list. Retain module names separately for audit output and add alias/mismatched-name regression coverage.
| '// (tsjs-prebid, served by the server) installs the trustedServer adapter', | ||
| '// onto the `window.pbjs` global this bundle populates and drives queue', | ||
| '// processing — this bundle intentionally does NOT call processQueue().', | ||
| "import 'prebid.js';", |
There was a problem hiding this comment.
P2 — No test executes the new production artifacts together.
The external-bundle test only searches output text, while shim tests preinstall a complete mocked window.pbjs; browser integration configuration disables Prebid. CI therefore never proves that this generated entry populates the public API consumed by the real server-served shim.
Add a JSDOM or browser test that builds and evaluates both production outputs, verifies the manifest and public API, asserts exactly one trustedServer registration and request wrapper, and exercises one transformed /auction request.
The APS adapter spec loads the external Prebid bundle and assumed it carries the tsjs shim (and the trustedServer adapter) inside it. Once the shim is decoupled (PR #967) the external bundle is pure Prebid.js, no adapter ever registers, and the in-page auction times out. Detect the decoupled world through the window.__tsjs_prebid_bundle manifest stamp and load dist/tsjs-prebid.js after the external bundle, matching the script order the server actually serves. On a coupled bundle the stamp is absent and the spec behaves exactly as before.
3 participants
Summary
The prebid tsjs shim (the
trustedServeradapter,requestBidswrapper, and Prebid config glue) was compiled into the external Prebid bundle:build-prebid-external.mjsused the shim as its entry point and importedprebid.jsplus internal modules directly. Any shim change therefore required rebuilding and re-uploading the external bundle and repointingexternal_bundle_sha256/external_bundle_sri— server deploys alone could never change client behavior.This PR separates the two artifacts:
userId, and the selected--adapters/--user-id-modules— no shim code. It stamps a manifest onwindow.__tsjs_prebid_bundle({adapters, userIdModules}) and intentionally does not callprocessQueue(); the shim keeps driving queue processing after it installs the adapter, preserving the existing ordering semantics.build-all.mjsno longer excludes prebid, and the Rust registration switches.without_js()→.with_deferred_js(), so the shim ships astsjs-prebidvia the/static/tsjs=route. It uses thewindow.pbjsglobal (the head-injected stub object that Prebid.js mutates in place) with a type-onlyimport typefor typing, and fails loudly if the external bundle did not load (noregisterBidAdapteron the global).adapterManager.getBidAdapter(an internal import) is replaced by validatingclient_side_biddersagainst the stamped manifest; the previously bundled consent/userId module imports move into the external bundle entry.Why
Shim fixes should reach production with a normal server deploy. With this split, the external bundle only changes when the Prebid.js version or the adapter/user-ID selection changes.
Verification
cargo fmt --all -- --check-D warningscargo testfor core + fastly (1658 + 109), axum, cloudflare, spin, and the parity integration suite (13)npx vitest run(412 tests) andnpm run formattsjs-prebid.jsshim (~23 KB, no Prebid core markers) and the pure external bundle (reproducible content hash across builds)window.pbjs(v10.26.0) and stamps the manifest, the deferred shim registers thetrustedServeradapter and wrapsrequestBids, server-side/auctionand client-side bidders both return bids, andhb_*targeting reaches every GPT slot with zero Prebid console errorsDeployment note
The currently deployed external bundle contains the old baked-in shim, so this change and a regenerated pure bundle should roll out together: upload the new bundle, update
external_bundle_sha256/external_bundle_sri, and deploy the server. Running the new server against the old bundle would install the adapter twice.Old-architecture guard tests (asserting the shim must not be served as embedded tsjs) are inverted to assert the new behavior.