✨ enable partial view updates by default for CDN users without a proxy - #5000
Draft
mormubis wants to merge 9 commits into
Draft
✨ enable partial view updates by default for CDN users without a proxy#5000mormubis wants to merge 9 commits into
mormubis wants to merge 9 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: f99fe23 | Docs | View more details | Give us feedback! |
Bundles Sizes Evolution
|
mormubis
marked this pull request as draft
August 31, 2026 10:40
Reverts the getSdkSetup accessor and its browser-core export. rum-core now declares __BUILD_ENV__SDK_SETUP__ locally, like the eight existing __BUILD_ENV__SDK_VERSION__ consumers. Drops the four npm-branch tests, which are unreachable under Karma because webpack.base.ts pins setup: 'cdn'. Also fixes prettier on the beta_enable_view_updates line.
Both declares of __BUILD_ENV__SDK_SETUP__ now read 'npm' | 'cdn', matching the setup argument of getBuildEnvDefines. With the previous 'string' type a typo like === 'CDN' would compile and silently disable the feature.
mormubis
commented
Aug 31, 2026
| trackLongTasks: { type: 'boolean', default: true, strict: false }, | ||
| trackViewsManually: { type: 'boolean', default: false, strict: false }, | ||
| betaEnableViewUpdates: { type: 'boolean', default: false }, | ||
| betaEnableViewUpdates: { type: 'boolean' }, |
Contributor
Author
There was a problem hiding this comment.
I removed the default. With default: false you can't tell if the user passed false or didn't pass anything, both end up as false after validation. So the default is resolved in validateAndBuildRumConfiguration instead.
The intersection already resolves it: (boolean | undefined) & boolean is boolean, and the required member wins over the optional one. Only allowedTracingUrls needs the Omit, because its schema type and resolved type are incompatible.
sdk_setup is merged in via combine() and cast to TelemetryEvent['telemetry'], so the type is never checked there. The narrow only pays off in rum-core where the value is compared against 'cdn'. Leaving telemetry.ts untouched avoids a declare that can go stale if a third setup is ever added.
…global The narrow only guarded a typo in === 'cdn', which the tests already catch. string matches the telemetry declare and the eight __BUILD_ENV__SDK_VERSION__ ones.
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
1 participant
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.
Motivation
Partial view updates have been behind
betaEnableViewUpdatessince #4833. This is the Sept 7 step of the Road to GA: enable it by default where we know it's safe.CDN users always run the latest bundle, so we can roll back by shipping a new version. npm users pin a version, so they opt in explicitly.
proxyusers stay off too, a proxy may not forward theview_updateevent type yet.Changes
betaEnableViewUpdatesdefaults totrueon CDN builds withoutproxy, andfalseeverywhere else. An explicit value from the user always wins.The schema
defaultonly takes static values, so I removed it from the field and resolve it invalidateAndBuildRumConfigurationinstead, next to the other post-processed options. That's the part worth a look: withdefault: falseyou can't tell "passedfalse" from "passed nothing", both come out asfalse. Without the default an unset option isundefined, so??can fill it in and an explicitfalsesurvives.Telemetry now reports the resolved value instead of the raw one. Otherwise every defaulted user shows up as
undefinedand we can't follow the rollout.CDN vs npm comes from
__BUILD_ENV__SDK_SETUP__, declared locally inconfiguration.tslike the other build-env consumers do. Unit tests are bundled ascdn, so the npm side of the default isn't covered by unit tests, only by e2e and by reading the code.Test instructions
yarn devand open the sandbox. It setsproxy: '/proxy', so the RUM configuration telemetry should reportbeta_enable_view_updates: false.proxyinsandbox/index.htmland reload. It should now betrue. Requests fail auth without the proxy, that's expected, the configuration event still goes out.yarn test:unit --spec packages/browser-rum-core/src/domain/configuration/configuration.spec.tsfor the proxy/no-proxy and explicit-value cases.Checklist
proxy, so they cover the disabled path.