Skip to content

♻️ move pure utilities to @datadog/js-core/util - #4994

Open
BenoitZugmeyer wants to merge 5 commits into
mainfrom
benoit/move-pure-utils-to-js-core
Open

♻️ move pure utilities to @datadog/js-core/util#4994
BenoitZugmeyer wants to merge 5 commits into
mainfrom
benoit/move-pure-utils-to-js-core

Conversation

@BenoitZugmeyer

@BenoitZugmeyer BenoitZugmeyer commented Aug 27, 2026

Copy link
Copy Markdown
Member

Motivation

Move more utilities to js-core. The goal is to eventually move batch into js-core, and this is a step toward it. All those utilities are dependencies of batch. It's not the only ones, but this PR is large enough.

Changes

  • byteUtils (computeBytesCount, concatBuffers, ONE_KIBI_BYTE, ONE_MEBI_BYTE, Uint8ArrayBuffer) moved to @datadog/js-core/util; spec moved alongside.
  • Context types (Context, ContextValue, ContextArray) moved to @datadog/js-core/util.
  • mockable (mockable, mockableReplacements) moved to @datadog/js-core/util.
  • jsonStringify (jsonStringify, detachToJsonMethod, ObjectWithToJsonMethod) moved to @datadog/js-core/util; spec moved alongside. The noop dependency is inlined to avoid pulling browser-specific utilities into js-core.
  • Uint8ArrayBuffer simplified from a custom interface to Uint8Array<ArrayBuffer>.
  • All consumers (browser-core internals, browser-logs, browser-rum, browser-rum-core, browser-rum-slim, browser-worker, browser-debugger, developer-extension, e2e tests) now import directly from @datadog/js-core/util instead of going through browser-core re-exports.
  • @datadog/js-core added as a dependency of @datadog/browser-worker.

Test instructions

This is a pure refactor with no observable behaviour change. All existing unit tests cover the moved code. To sanity-check the SDK still initialises and sends data correctly:

  1. Run yarn dev and open http://localhost:8080.
  2. Check the Network tab — RUM and Logs requests should still be sent to the intake endpoints on the configured interval.
  3. Navigate away from the page and confirm a flush request is sent on beforeunload/visibilitychange.

Checklist

  • Tested locally
  • Tested on staging
  • Added unit tests for this change.
  • Added e2e/integration tests for this change.
  • Updated documentation and/or relevant AGENTS.md file

@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Aug 27, 2026

Copy link
Copy Markdown

Tests

All CI checks and tests passed.

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 77.01% (+0.00%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 3b565c9 | Docs | View more details | Give us feedback!

@BenoitZugmeyer
BenoitZugmeyer force-pushed the benoit/move-pure-utils-to-js-core branch from 8244c7f to 11970f7 Compare August 27, 2026 15:45
@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Aug 27, 2026

Copy link
Copy Markdown

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 182.02 KiB 182.02 KiB -3 B -0.00%
Rum Profiler 8.43 KiB 8.43 KiB 0 B 0.00%
Rum Recorder 25.32 KiB 25.32 KiB 0 B 0.00%
Logs 57.96 KiB 57.98 KiB +16 B +0.03%
Rum Salesforce N/A 140.10 KiB N/A N/A N/A
Rum Slim 140.08 KiB 140.09 KiB +15 B +0.01%
Worker 22.96 KiB 22.96 KiB 0 B 0.00%
Rum Shopify N/A 206.82 KiB N/A N/A N/A
Rum-shopify Profiler N/A 8.43 KiB N/A N/A N/A
Rum-shopify Recorder N/A 3.74 KiB N/A N/A N/A

@BenoitZugmeyer
BenoitZugmeyer force-pushed the benoit/move-pure-utils-to-js-core branch 4 times, most recently from 66c96ed to 40b080d Compare August 28, 2026 09:03
@BenoitZugmeyer
BenoitZugmeyer force-pushed the benoit/move-pure-utils-to-js-core branch from 40b080d to 060a320 Compare August 31, 2026 08:20
@BenoitZugmeyer
BenoitZugmeyer force-pushed the benoit/move-pure-utils-to-js-core branch from 060a320 to 80ae39a Compare September 2, 2026 09:49
Base automatically changed from benoit/move-monitor-to-js-core to main September 3, 2026 15:46
@BenoitZugmeyer
BenoitZugmeyer force-pushed the benoit/move-pure-utils-to-js-core branch 2 times, most recently from f98fe12 to 5b800e9 Compare September 4, 2026 12:16
@BenoitZugmeyer
BenoitZugmeyer marked this pull request as ready for review September 4, 2026 12:16
@BenoitZugmeyer
BenoitZugmeyer requested review from a team as code owners September 4, 2026 12:16
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-04T13:40:02.151714Z 3b565c9 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

const restoreValuePrototypeToJson = detachToJsonMethod(Object.getPrototypeOf(value))

P2 Badge Handle objects without a prototype before detaching toJSON

issue: When the new public jsonStringify API receives Object.create(null), Object.getPrototypeOf(value) returns null, which is passed to detachToJsonMethod and dereferenced before the try block. This throws even though native JSON.stringify accepts null-prototype objects and the function accepts unknown; guard the missing prototype or include this setup in the protected path.

AGENTS.md reference: packages/js-core/AGENTS.md:L33-L44

ℹ️ 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 thread packages/browser-worker/package.json Outdated
Comment thread packages/js-core/src/entries/util.ts
Move `computeBytesCount`, `concatBuffers`, `ONE_KIBI_BYTE`, `ONE_MEBI_BYTE`,
and the `Uint8ArrayBuffer` type from `browser-core/src/tools/utils/byteUtils`
to `js-core/src/util/byteUtils`. The spec file moves alongside.

All consumers (browser-core internals, browser-rum, browser-rum-core,
browser-worker) now import directly from `@datadog/js-core/util` instead of
going through browser-core re-exports.
Move `Context`, `ContextValue`, and `ContextArray` type definitions from
`browser-core/src/tools/serialisation/context` to
`js-core/src/util/context`.

All consumers (browser-core internals, browser-logs, browser-rum,
browser-rum-core, browser-rum-slim, developer-extension, e2e tests) now
import directly from `@datadog/js-core/util`.
Move `mockable` and `mockableReplacements` from
`browser-core/src/tools/mockable` to `js-core/src/util/mockable`.

All consumers (browser-core internals, browser-logs, browser-rum,
browser-rum-core, browser-debugger, developer-extension) now import
directly from `@datadog/js-core/util`.
Move `jsonStringify`, `detachToJsonMethod`, and the `ObjectWithToJsonMethod`
type from `browser-core/src/tools/serialisation/jsonStringify` to
`js-core/src/util/jsonStringify`. The spec file moves alongside.

The `noop` dependency (previously imported from browser-core's
`functionUtils`) is inlined as a private function to avoid pulling
browser-specific utilities into js-core.

All consumers (browser-core internals, browser-rum-core) now import
directly from `@datadog/js-core/util`.
@BenoitZugmeyer

Copy link
Copy Markdown
Member Author

🤖 Regarding the P2 Object.create(null) note on jsonStringify: the null-prototype behavior predates this PR — the function was moved verbatim from browser-core. I'll address it in a follow-up rather than scope-creep this move.

@BenoitZugmeyer
BenoitZugmeyer force-pushed the benoit/move-pure-utils-to-js-core branch from 5b800e9 to e91981b Compare September 4, 2026 13:27

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e91981b3c3

ℹ️ 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 thread packages/js-core/src/util/context.ts
Add TSDoc comments to the utilities moved from browser-core to js-core
(byteUtils, Context types, mockable, jsonStringify, detachToJsonMethod,
ObjectWithToJsonMethod) so they appear as documented exports in the
API Extractor report instead of `(undocumented)`.
@BenoitZugmeyer
BenoitZugmeyer force-pushed the benoit/move-pure-utils-to-js-core branch from e91981b to 3b565c9 Compare September 4, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant