♻️ Migrate browser-core monitoring/display to @datadog/js-core#4758
♻️ Migrate browser-core monitoring/display to @datadog/js-core#4758thomas-lebeau wants to merge 8 commits into
Conversation
Bundles Sizes Evolution
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 7980483 | 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
|
/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
| 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() |
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".
| "./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 👍 / 👎.
There was a problem hiding this comment.
The release job will also release js-core
thomas-lebeau
force-pushed
the
thomas.lebeau/add-browser-core-agents-md
branch
from
b1cee2a to
05ff0f1
Compare
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
2 participants
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