♻️ [js-core] Schema-driven configuration validation across browser-core, browser-logs, and browser-rum-core#4798
♻️ [js-core] Schema-driven configuration validation across browser-core, browser-logs, and browser-rum-core#4798thomas-lebeau wants to merge 9 commits into
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 0690d15 | Docs | Datadog PR Page | Give us feedback! |
Bundles Sizes Evolution
|
thomas-lebeau
force-pushed
the
worktree-js-core-configuration
branch
7 times, most recently
from
2d8531b to
0e608e6
Compare
thomas-lebeau
force-pushed
the
worktree-js-core-configuration
branch
4 times, most recently
from
9c41061 to
629c7b1
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 629c7b13dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sessionPersistence: { | ||
| type: 'enum', | ||
| values: ['cookie', 'local-storage', 'memory'] as const, | ||
| multiple: true, | ||
| strict: false, | ||
| }, |
There was a problem hiding this comment.
With strict: false on this public option, an explicit unrecognized sessionPersistence value from plain JavaScript (for example a typo) is dropped by schema validation and becomes undefined, so selectSessionStoreStrategyType() falls back to the default cookie/memory strategy. Before this change the raw invalid value reached the strategy selector and no session store was selected, so this can start tracking with cookies even though the caller explicitly configured an invalid persistence mode; make this field strict or otherwise preserve the invalid value so it does not silently default.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Technically a breaking change, but I don't think anyone willingly use invalid session persistence to prevent the SDK from starting. I think it's fine
| export const DEFAULT_REQUEST_ERROR_RESPONSE_LENGTH_LIMIT = 32 * ONE_KIBI_BYTE | ||
| export const LOGS_SCHEMA = { | ||
| ...BROWSER_CORE_SCHEMA, | ||
| forwardErrorsToLogs: { type: 'boolean', default: true, strict: false }, |
There was a problem hiding this comment.
For plain JavaScript users who pass a falsy non-boolean value such as 0 or '', this strict: false boolean coercion turns forwardErrorsToLogs off. The previous implementation, and the test that was removed, treated every value except the literal false as enabled via initConfiguration.forwardErrorsToLogs !== false, so these configurations now silently stop forwarding uncaught/runtime/network errors. This option needs the old !== false behavior rather than generic !!value coercion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Also technically a small breaking change but this makes the behaviour more predictable. For example forwardErrorsToLogs: 0 is probably not expected to actually forward errors to logs
| strict: false, | ||
| default: DEFAULT_PROPAGATOR_TYPES, |
There was a problem hiding this comment.
When an allowedTracingUrls object has invalid propagatorTypes (for example a typo array like ['traceContext'] or a non-array value), this nested strict: false field falls back to DEFAULT_PROPAGATOR_TYPES, so the SDK injects Datadog/tracecontext headers for that URL. Previously invalid propagators produced no tracing headers or caused the option to be ignored, which avoids unexpected CORS preflights and unintended trace propagation; keep invalid propagatorTypes ignored instead of defaulting them to active headers.
Useful? React with 👍 / 👎.
thomas-lebeau
force-pushed
the
worktree-js-core-configuration
branch
from
629c7b1 to
ec72176
Compare
Move buildCookieOptions and CookieConfiguration out of configuration.ts into cookie.ts where they logically belong. - CookieConfiguration now lives in cookie.ts with optional boolean fields - buildCookieOptions accepts CookieConfiguration instead of InitConfiguration - configuration/index.ts re-exports buildCookieOptions from cookie.ts - cookie.spec.ts covers all four buildCookieOptions scenarios
Introduces a declarative, type-safe configuration engine in a new @datadog/js-core/configuration sub-path entry point. Field types: StringField, PercentageField, BooleanField, SiteField, MatchOptionField, EnumField, UnionField, SchemaField, FunctionField. Key features: - InferredConfig<S> derives the output type from the schema definition, making it impossible for the interface to drift from the schema - strict: false mode for backward-compatible validation fallback - FunctionField with signature phantom field for precise type inference - ConfigurationSchema as an interface for better type ergonomics - Per-field auto-generated error messages via display.error Also registers the package in tsconfig path aliases and eslint side-effect allowlist.
Replaces the hand-written Configuration interface and per-field validators with a declarative BROWSER_CORE_SCHEMA. Configuration is now inferred via InferredConfig<typeof BROWSER_CORE_SCHEMA> — impossible to drift. Key changes: - validateAndBuildConfiguration is now a thin wrapper: passes display.error to the schema engine so all field errorMessages surface as display.error calls - trackingConsent uses the object-form enum (INVALID on explicit invalid value) - source uses the array-form enum (defaults to 'browser' on invalid/missing) - sessionPersistence uses enum + multiple: true, normalising 'cookie' → ['cookie'] - allowedTrackingOrigins uses match-option + multiple: true - The three cookie flags (useSecureSessionCookie, usePartitionedCrossSiteSessionCookie, trackSessionAcrossSubdomains) are now first-class schema fields; cookieOptions is removed from Configuration — sessionInCookie.ts calls buildCookieOptions() directly - BROWSER_CORE_SCHEMA exported so browser-logs and browser-rum-core can extend it - normalizePersistenceList simplified — sessionPersistence is always SessionPersistence[] | undefined after schema validation - isAllowedTrackingOrigins moved to the init layer (preStartLogs/preStartRum)
Replaces hand-written LogsConfiguration validation with a declarative LOGS_SCHEMA that extends BROWSER_CORE_SCHEMA. - forwardErrorsToLogs: boolean field with default true - forwardConsoleLogs / forwardReports: enum fields with allowAll: true - Removes validateAndBuildForwardOption helper (schema handles it) - isAllowedTrackingOrigins check moved to preStartLogs init layer
Replaces hand-written RumConfiguration validation with a declarative RUM_SCHEMA that extends BROWSER_CORE_SCHEMA. Key changes: - allowedTracingUrls / excludedActivityUrls: union + multiple fields, single string/regex now normalises to array instead of erroring - propagatorTypes: invalid values fall back to defaults - trackFeatureFlagsForEvents: invalid items fall back to [] - RumConfiguration type inferred from schema with manual overrides for complex fields (beforeSend, plugins, allowedTracingUrls) - isAllowedTrackingOrigins check moved to preStartRum init layer
- Cast DEFAULT_CONFIG to Configuration in allowedTrackingOrigins.spec.ts - Refactor sessionContext.spec.ts: replace beforeEach with setup() factory to avoid shared mutable state across tests - Fix recordingScope.specHelper.ts: spread configuration instead of casting - Fix trackInput/trackMutation specs: recreate scope instead of mutating configuration.defaultPrivacyLevel in place - Export DEFAULT_REQUEST_ERROR_RESPONSE_LENGTH_LIMIT from networkErrorCollection and update e2e import path accordingly
Register src/entries/configuration.ts as a typedoc entry point and add the generated API Extractor report for the js-core configuration API.
- Document Optionality, Multiple, Strict, and all FieldDef variants (StringField, PercentageField, BooleanField, SiteField, MatchOptionField, EnumField, UnionField, SchemaField, FunctionField) - Document ConfigurationSchema, InferredConfig, and validateAndBuildConfiguration - Regenerate configuration.api.md to reflect the newly documented exports (removes "(undocumented)" tags)
thomas-lebeau
force-pushed
the
worktree-js-core-configuration
branch
from
ec72176 to
34b7bf5
Compare
- Split InferredConfig into required and optional key sets so fields without `required: true` or a `default` become optional properties instead of always-present `| undefined` properties - Lets consumers building configuration object literals by hand (e.g. test fixtures) omit fields instead of writing `field: undefined` - Update API report and add a compile-time test verifying required fields stay required while optional fields can be omitted
1 participant
Motivation
Configuration validation in
browser-core,browser-logs, andbrowser-rum-corewas previously done imperatively — each package had its own ad-hoc validation logic with repetitive checks, inconsistent error handling, and no shared abstraction.This PR introduces a schema-driven configuration system in
@datadog/js-corethat declaratively describes fields, their types, defaults, and validation rules. Browser packages then derive theirConfigurationtypes and validation functions from these schemas.Changes
@datadog/js-core: Addconfigurationentry point withvalidateAndBuildConfiguration,InferredConfig, and field definition types (StringField,BooleanField,PercentageField,SiteField,MatchOptionField,EnumField,CustomField). Includes anINVALIDsentinel value that custom validators can return to fail the whole configuration.browser-core: Replace imperativevalidateAndBuildConfigurationand manualConfigurationinterface withBROWSER_CORE_SCHEMA+InferredConfig<typeof BROWSER_CORE_SCHEMA>. Cookie-option fields (useSecureSessionCookie, etc.) are moved into the schema.buildCookieOptionsnow takes a typedCookieConfigurationrather than raw init config.browser-logs: ReplaceLogsConfigurationinterface and imperative validation withLOGS_SCHEMAthat spreadsBROWSER_CORE_SCHEMAand addsforwardErrorsToLogs,forwardConsoleLogs,forwardReports, andrequestErrorResponseLengthLimit.browser-rum-core: Same pattern —RUM_SCHEMAspreadsBROWSER_CORE_SCHEMAand adds all RUM-specific fields.Test instructions
Checklist