♻️ move batching transport stack to @datadog/js-core#4856
Draft
BenoitZugmeyer wants to merge 11 commits into
Draft
♻️ move batching transport stack to @datadog/js-core#4856BenoitZugmeyer wants to merge 11 commits into
BenoitZugmeyer wants to merge 11 commits into
Conversation
Move byteUtils, context types, jsonStringify, objectValues and isServerError to js-core — they have no browser-specific dependencies and are needed by the transport modules that will follow in subsequent commits. Browser-core files become thin re-exports so all existing relative imports keep working without touching every consumer.
Both classes have no browser-specific dependencies. queueMicrotask moves to js-core as a pure scheduling helper (no monitor wrapping — callers are responsible for error handling at their layer). browser-core/tools/observable.ts becomes a thin re-export. browser-core/tools/queueMicrotask.ts is deleted: its only consumer was observable.ts, which now lives in js-core.
Both files already used globalObject from js-core; moving them avoids a circular path when transport modules (flushController, sendWithRetryStrategy) migrate to js-core in upcoming commits. The js-core timer drops the monitor() wrapping — callers are responsible for error handling at their own layer. browser-core/tools/timer.ts keeps its monitored wrappers but now imports getZoneJsOriginalValue from js-core. browser-core/tools/getZoneJsOriginalValue.ts becomes a thin re-export.
…ransport Payload, HttpResponse, BandwidthStats and HttpRequestEvent move to a new payload.ts — they have no browser-specific dependencies and are needed by both sendWithRetryStrategy and (later) httpRequest. navigator.onLine becomes globalObject.navigator?.onLine for cross-environment safety. browser-core/transport/sendWithRetryStrategy.ts becomes a thin re-export. httpRequest.ts and its spec are updated to import the types from js-core.
PageExitReason and PageMayExitEvent move alongside it as pure types — the browser implementation (createPageMayExitObservable) stays in browser-core; flushController consumes the observable as an injected parameter so it needs no browser APIs. RECOMMENDED_REQUEST_BYTES_LIMIT moves to payload.ts (js-core) since it is a transport-layer constant used by both flushController and httpRequest. browser-core/transport/flushController.ts and browser-core/browser/pageMayExitObservable.ts become thin re-exports.
fetch and sendBeacon are browser-specific APIs; they become injectable strategies so createHttpRequest stays free of browser globals. browser-core/transport/httpRequest.ts keeps fetchStrategy and sendBeaconStrategy and wraps the js-core function by injecting them. The public signature (endpointBuilders, reportError, bytesLimit?) is unchanged for all existing callers.
createHttpRequest and pageMayExitObservable are now explicit parameters, removing the last browser-specific dependencies from the batching logic. display.warn is replaced by an injected warn function. browser-core/transport/batch.ts becomes a thin wrapper that wires up the browser defaults (createHttpRequest, createPageMayExitObservable, display.warn) and delegates to the js-core implementation. All existing callers are unchanged. isPageExitReason also moves to js-core/transport alongside PageExitReason.
… public API These were exported as stepping stones during the migration but have no legitimate external consumers: - sendWithRetryStrategy, newRetryState, RetryState, and the bandwidth / retry constants are internal machinery of createHttpRequest; remove them from @datadog/js-core/transport - FlushController is the internal return type of createFlushController; callers that needed the type can use ReturnType<typeof createFlushController> - browser-core's dead sendWithRetryStrategy.ts re-export shim is deleted (nothing ever imported from it) - sendWithRetryStrategy.spec.ts moves from browser-core to js-core so it can use internal imports; a minimal test/ helper (registerCleanupTask, mockClock, mockNavigator) is added to js-core for this
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: db9e0f1 | Docs | Datadog PR Page | Give us feedback! |
BenoitZugmeyer
force-pushed
the
benoit/move-transport
branch
from
fa999e3 to
db9e0f1
Compare
Bundles Sizes Evolution
|
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
The
@datadog/js-corepackage is meant to hold runtime-agnostic SDK utilitiesshared across all Datadog JS SDKs. However, the entire batching transport stack
(
createBatch,createHttpRequest,createFlushController, and all theirdependencies) lived exclusively in
@datadog/browser-core, making itunavailable to non-browser consumers such as the Worker or future server-side
SDK targets.
This PR moves the complete transport stack into
@datadog/js-core, withbrowser-specific concerns (fetch, sendBeacon, DOM events) remaining in
browser-coreas injectable strategies.Changes
byteUtils,jsonStringify,context,objectValues,isServerError) moved to@datadog/js-core/util; browser-core re-exports.mockablemoved to@datadog/js-core/util.Observable,BufferedObservable,mergeObservables,queueMicrotaskmoved to
@datadog/js-core/util;queueMicrotaskdrops themonitor()wrapper (callers handle error wrapping at their own layer).
Encoder/createIdentityEncodermoved to@datadog/js-core/transport.timer/getZoneJsOriginalValuemoved to@datadog/js-core/util;js-core timer has no
monitor()wrapping; browser-core keeps monitored wrappers.sendWithRetryStrategyandPayloadtypes moved to@datadog/js-core/transport;navigator.onLineaccess goes throughglobalObject.flushControllermoved to@datadog/js-core/transport;PageExitReasonand
PageMayExitEventmove with it as pure types;pageMayExitObservableis injected as a parameter.
createHttpRequestmoved to@datadog/js-core/transportwith injectablesendStrategy/sendOnExitStrategy; browser-core keepsfetchStrategyandsendBeaconStrategyand wraps the js-core factory.createBatchmoved to@datadog/js-core/transport;requestandpageMayExitObservableare injected parameters;display.warnbecomes aninjected
warnfunction; browser-core wrapper preserves the existing callsignature for all current callers.
sendWithRetryStrategy,newRetryState,RetryState,internal constants,
createFlushController, andFlushControllerareremoved from js-core's public transport entry — they are internal details.
Their specs move to js-core alongside the implementation.
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:
yarn devand openhttp://localhost:8080.intake endpoints on the configured interval.
beforeunload/visibilitychange.Checklist