♻️ Migrate browser-core monitoring/display to @datadog/js-core#4758
Open
thomas-lebeau wants to merge 20 commits into
Open
♻️ Migrate browser-core monitoring/display to @datadog/js-core#4758thomas-lebeau wants to merge 20 commits into
thomas-lebeau wants to merge 20 commits into
Conversation
Bundles Sizes Evolution
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 2ebe098 | Docs | Datadog PR Page | Give us feedback! |
thomas-lebeau
force-pushed
the
thomas.lebeau/add-browser-core-agents-md
branch
2 times, most recently
from
ccba31a to
af7a15d
Compare
Collaborator
Author
|
/trigger-ci |
|
View all feedbacks in Devflow UI.
Branch
Started pipeline #118369723 |
thomas-lebeau
force-pushed
the
thomas.lebeau/add-browser-core-agents-md
branch
3 times, most recently
from
18ce6e4 to
2123255
Compare
thomas-lebeau
changed the base branch from
main
to
worktree-migrate-build-to-tsdown
thomas-lebeau
commented
Jun 15, 2026
Comment on lines
+46
to
+48
| it('catches monitored errors but does not report them (no collection callback yet)', () => { | ||
| expect(() => candidate.notMonitoredThrowing()).toThrowError('not monitored') | ||
| expect(() => candidate.monitoredThrowing()).toThrowError('monitored') | ||
| expect(() => candidate.monitoredThrowing()).not.toThrowError() |
Collaborator
Author
There was a problem hiding this comment.
note: this fix an behaviour inconsistency between the @monitor decorator and the monitor() function
![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: b1cee2adf9
ℹ️ 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".
Comment on lines
+12
to
+16
| "./monitor": { | ||
| "import": "./esm/entries/monitor.mjs", | ||
| "require": "./cjs/entries/monitor.js", | ||
| "types": "./cjs/entries/monitor.d.ts" | ||
| }, |
There was a problem hiding this comment.
issue: These new public subpaths are being added while @datadog/js-core still declares version 0.0.1, and the browser packages pin that exact version. In any release where 0.0.1 already exists, the publish job can tolerate/skip republishing js-core, but browser-core now imports @datadog/js-core/monitor; consumers would then install the old tarball and hit a module-not-found error. Please bump @datadog/js-core and update the dependent pins when adding these exports.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
The release job will also release js-core
gogusarov
approved these changes
Jun 15, 2026
thomas-lebeau
force-pushed
the
thomas.lebeau/add-browser-core-agents-md
branch
from
b1cee2a to
05ff0f1
Compare
Replace the TypeScript compiler API (`ts.createProgram`/`program.emit`) with tsdown (Rolldown-based) for transpiling package modules to CJS and ESM. Declarations are still emitted by `tsc` (`--emitDeclarationOnly`): Rolldown's declaration bundler restructures modules in ways that break older TypeScript consumers (inline `type` modifiers, `.js` import extensions, rewritten re-exports that trip `isolatedModules` on const enums). Keeping tsc for `.d.ts` makes the declaration output identical to before, preserving the TS 3.8+ compatibility matrix. Declarations are emitted only next to the CJS output, which every package's `types` field already points at. oxc lowers the one decorator in the codebase (`@monitored`) to a helper imported from `@oxc-project/runtime` and cannot inline it. To avoid adding a runtime dependency, the used helpers are vendored into the output and their imports rewritten to local copies. Build-time constants (`__BUILD_ENV__*__`) are inlined via tsdown's `define` option instead of post-processing emitted files. ESM output uses the `.mjs` extension, so the `--esm-type-module` flag (which wrote `esm/package.json`) is no longer needed.
- Replace @monitored decorator on Logger.logImplementation with callMonitored to avoid tsdown emitting oxc runtime helpers - Remove vendorOxcRuntimeHelpers workaround from build-package.ts - Drop @oxc-project/runtime devDependency and its LICENSE-3rdparty.csv entry
Declare @datadog/browser-worker as a devDependency of browser-rum and switch the build command to --topological-dev so yarn's workspace topological ordering guarantees browser-worker is built before browser-rum. Previously, the parallel build could start browser-rum before browser-worker produced bundle/worker.js, causing a flaky ENOENT error in getBuildEnvValue.
- add getEntries() to pick entry globs based on whether the package uses src/index.ts + src/entries or top-level src modules - set tsdown `root` to ./src so unbundle output mirrors the src layout (e.g. src/entries/main.ts -> cjs/entries/main.js) instead of being flattened to the entries' common ancestor - drop the `deps.neverBundle` external override
- bundles no longer depend on each other's build output, so parallel ordering is unnecessary after the tsdown migration
- move js-core time.ts to src/entries/time.ts so its public surface lives under src/entries - drop getEntries() heuristic; always derive tsdown entries from src/index.ts + src/entries/*.ts - update js-core exports, time/package.json, and tsconfig path to the new location
- update js-core AGENTS.md: sub-paths live under src/entries/, ESM output uses .mjs (no esm/package.json needed), and tsconfig paths point at src/entries - add TODO in build-package.ts explaining why declarations use emitDeclarations instead of tsdown dts (needs TS 4.7+ for bundler resolution and inline type)
thomas-lebeau
force-pushed
the
worktree-migrate-build-to-tsdown
branch
from
9f852fb to
edc5440
Compare
Documents that browser-core has no semver stability guarantee and breaking changes are allowed, so review warnings about removed exports or signature changes in this package should be disregarded.
- monitor: createMonitor(display, onMonitorErrorCollected) factory exposing a documented Monitor interface (monitor, callMonitored, monitored, monitorError) - util: createDisplay + setDebugMode, with a debug-gated ifDebugEnabled facet on Display; util is a folder with a barrel index - wire up exports, legacy fallbacks, and tsconfig paths for both sub-paths
- monitor.spec.ts: adapts the browser-core monitor spec to the createMonitor factory API, injecting a fake Display and a spy error callback - util/display.spec.ts: covers each console method (always-on + ifDebugEnabled gating), asserting the prefixed console call; resets debug mode with afterEach - make display.ifDebugEnabled delegate to the display's public methods so the gating is observable in tests (and respects method overrides) - export originalConsoleMethods as a test seam (kept out of the util barrel) - tidy monitor JSDoc to satisfy jsdoc lint rules
- Move debug-mode state out of `display.ts` into a dedicated `debug.ts` (`setDebugMode`/`getDebugMode`) and drop the `ifDebugEnabled` facet from `Display` - Export `ConsoleApiName`, `globalConsole`, `originalConsoleMethods` from the `@datadog/js-core/util` barrel - Re-implement browser-core `monitor`/`display` on top of `@datadog/js-core` (`createMonitor`, `createDisplay`) and update consumers across browser-core, browser-logs, browser-rum-core and browser-debugger to import from js-core - Whitelist `@datadog/js-core/util` and `/monitor` as side-effect-free
Core monitor behavior (decorator, wrapper, callMonitored, debug logging) is now tested in @datadog/js-core. This spec is reduced to the two cases unique to the browser-core wrapper: errors are silently swallowed before startMonitorErrorCollection, and reported to the callback after.
- Sync workspace dependency version references in yarn.lock - Matches the version bumps from packages/browser-core and js-core
thomas-lebeau
force-pushed
the
thomas.lebeau/add-browser-core-agents-md
branch
from
1bef825 to
7980483
Compare
|
|
||
| let onMonitorErrorCollected: undefined | ((error: unknown) => void) | ||
| let debugMode = false | ||
| export { setDebugMode } |
Member
There was a problem hiding this comment.
suggestion: import setDebugMode from js-core where it is used
Comment on lines
+113
to
+115
| descriptor.value = function (this: ThisParameterType<T>, ...args: Parameters<T>): ReturnType<T> { | ||
| return monitor(originalMethod).apply(this, args) as ReturnType<T> | ||
| } as T |
Member
There was a problem hiding this comment.
nitpick: I know you just ported the original implementation, but I realize this could probably be simplified as descriptor.value = monitor(originalMethod)
| @@ -0,0 +1,68 @@ | |||
| /** | |||
| * Keep references on console methods to avoid triggering patched behaviors | |||
Member
There was a problem hiding this comment.
nitpick: This comment seems misplaces?
…wser-core-agents-md
- Remove `export { setDebugMode }` from browser-core's monitor.ts
- Update init.ts and segment.spec.ts to import setDebugMode directly
from @datadog/js-core/util instead of @datadog/browser-core
- Remove setDebugMode from browser-core's public index.ts exports
- Simplify `monitored` decorator in js-core to assign monitor(fn)
directly instead of wrapping in an extra closure
- Move the console-patching comment in display.ts closer to the
relevant code it describes
mormubis
approved these changes
Jun 16, 2026
Collaborator
Author
|
/to-staging |
|
View all feedbacks in Devflow UI.
Commit 2ebe098f1a will soon be integrated into staging-25. Use ⏳ Processing... |
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
4 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.
Motivation
Continue extracting runtime-agnostic primitives out of
browser-coreinto the stable@datadog/js-corepackage. This moves the monitoring and display utilities so they can be shared across Datadog JavaScript SDKs.Changes
monitorandutilsub-paths to@datadog/js-core, exposingmonitor,display, anddebughelpers.browser-coremonitoring/display tooling to consume the@datadog/js-coreimplementations.@datadog/js-coremonitor and display utilities.AGENTS.mddocumentation for thebrowser-corepackage and update@datadog/js-coredocs.Test instructions
yarn test:unit --spec packages/js-core/src/monitor.spec.tsyarn test:unit --spec packages/js-core/src/util/display.spec.tsyarn typecheckChecklist