Decouple the prebid tsjs shim from the bundled Prebid.js - #967
Open
aram356 wants to merge 9 commits into
Open
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
aram356
added a commit
that referenced
this pull request
Jul 31, 2026
aram356
requested review from
ChristianPavilonis and
prk-Jr
and removed request for
prk-Jr
July 31, 2026 08:17
prk-Jr
requested changes
Jul 31, 2026
prk-Jr
left a comment
prk-Jr
left a comment
Collaborator
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)
ChristianPavilonis
approved these changes
Jul 31, 2026
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
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.
aram356
added a commit
that referenced
this pull request
Jul 31, 2026
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.
aram356
added a commit
that referenced
this pull request
Aug 1, 2026
Brings in the #988 browser-spec fix through its PR lineage and, because #988 stacks on #963's head, refreshes rc's stale #963 absorption with the July 29-30 rework: - bid.meta second descriptor carrier and bidAccepted registration replacing the requestId stash (prebid shim and Rust provider) - Hardened APS auction delivery: sanitized publisher page identity (query/fragment stripped), delivery drop telemetry with dropped_winner_count/reasons, imp disposition counters - ProviderLaunchState/ProviderRequestOutcome orchestrator refactor with parse_state threading and Immediate outcomes - as_aps() Option accessor, fail-closed render-bridge stop, responsive slot-root helpers, case-insensitive APS exclusion tests Preserved rc-only systems the #963 branch predates: #956 opt-in creative processing (process_auction_creative, sanitize_creatives), #967 decoupled prebid shim (public markWinningBidAsUsed instead of prebid.js internals), #948/#912 GPT sync, #865 platform timeout canonicalization (restored at both launch paths and both mediator paths, with the duplicate backend-name pre/post-launch guards and their test suite ported to the new provider API), and the provider-validation startup checks.
aram356
added a commit
that referenced
this pull request
Aug 1, 2026
…c/july Adopts main's finalized #974 GPT diagnostics (standalone ts_console-gated module, finalize_response wiring, updated overlay/store/badges/binding) over rc's earlier absorbed copy, and #984's whole-package eslint gate (eslint . --max-warnings=0). Keeps rc-only content where the two lint passes collided: the #963/#967/#988 test rewrites in the prebid, APS, request, and ad_init suites, the GptSlotHandoff type, and the unexpected-origin-304 guard alongside main's diagnostics finalize call in publisher.rs.
Address review feedback on the shim/bundle seam: - Gate self-init on a loaded Prebid.js API: when the external bundle is missing, skip installRefreshHandler and user ID setup so publisher GPT refreshes keep their targeting instead of being cleared with no auction to refill them; cover the bail-out path with a test - Stamp registered bidder codes (including aliases) derived from prebid.js metadata and validate client_side_bidders against them, retaining module names for audit output - Make installPrebidNpm idempotent per page via a window.__tsjsPrebidShimInstalled sentinel - Validate the window-global bundle manifest shape before use - Warn once about an unstamped User ID manifest instead of once per configured module - Add a processQueue watchdog to the generated bundle entry so pbjs.que still drains if the shim artifact fails to load - Point the missing-adapter error at [integrations.prebid.bundle].adapters and ts prebid bundle - Assert the external bundle script precedes the deferred shim in processed HTML - Add an artifact integration test that builds and evaluates both production outputs together, plus a guard that the shim stays Prebid-free - Delete the unused generated-module placeholders and document the lockstep bundle/server rollout
Collaborator
Author
|
Review feedback addressed in cdff897 — inline replies on each thread. The summary-only items are covered too: the unused Verification: 556 JS tests, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.