Repository navigation
refactor(container-loader): export mixinDebugLogger as a free function - #28058
Merged
MarioJGMsoft merged 1 commit intoAug 24, 2026
Merged
Conversation
`DebugLogger` was exported only to serve as a namespace for a single static method. It is not re-exported from the package index and appears in no API report, and its private constructor meant no caller could hold an instance anyway -- `mixinDebugLogger` returns a `TelemetryLoggerExt` built by `createMultiSinkLogger`. Export that mixin directly instead and let the class become module private. The private constructor is no longer needed to prevent outside construction, so it becomes a normal one. No public API change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8a07af10-6ed8-4c13-bd58-176b5110a9d4
github-actions
Bot
added
area: tools
area: loader
Contributor
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (87 lines, 3 files), I've queued these reviewers:
How this works
|
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors container-loader to export mixinDebugLogger as a free function while preserving behavior and the public API.
Changes:
- Makes
DebugLoggermodule-private. - Updates both call sites to use
mixinDebugLogger. - Preserves logger and telemetry behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/loader/container-loader/src/loader.ts |
Updates the loader call site and import. |
packages/loader/container-loader/src/debugLogger.ts |
Exports the mixin function and privatizes the implementation class. |
packages/loader/container-loader/src/createAndLoadContainerUtils.ts |
Updates the container utility call site and import. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Bundle size comparisonBase commit: Notable changesNo bundles changed by ≥ 500 bytes parsed. Per-bundle deltas
|
Jason Hartman (jason-ha)
approved these changes
Aug 24, 2026
MarioJGMsoft
added a commit
that referenced
this pull request
Aug 25, 2026
#28044) ## Description Preparatory, non-breaking half of the `ITelemetryBaseLogger.send` breaking change, split out of #27982 per [review feedback](#27982 (review)). The breaking interface change stays in #27982, which is stacked on this PR. Internal plumbing only: - Every internal call site passes an explicit `LogLevel`. Sites that relied on the implicit default pass `LogLevel.essential`, preserving behavior. - Forwarding loggers now forward the level instead of dropping it and letting the inner logger silently re-default: `TaggedLoggerAdapter`, `EventAndErrorTrackingLogger`, `DevtoolsLogger`, `ConsoleVerboseLogger`, `TelemetryOptInLogger`, `FileLogger`, `BaseFileLogger`/`CSVFileLogger`, and the mocha test-run logger. Making the level explicit is what enables consistent filtering and sampling. This gets the repo there without touching public API — `logLevel` stays optional, so there are no API report changes and no changeset. Implementations still accept `undefined`, deliberately. Packages compiled before the parameter becomes required call `send(event)` with one argument, and will for as long as those versions are supported; an implementation assuming a defined value would silently drop those events. For the same reason, test loggers spliced into a real container declare `logLevel?` and forward it, while stubs that ignore the level don't declare the parameter at all. Part of #27595 (sub-issue of #23271). ## Reviewer Guidance The review process is outlined in [the pull request guidelines](https://github.com/microsoft/FluidFramework/blob/main/docs/content/Contributing/PR-Guidelines.md#guidelines). - Any forwarding logger still dropping the level? > The `DebugLogger` export refactor suggested in review was split out into #28058, which has merged. This branch is rebuilt on top of it and no longer touches `debugLogger.ts`. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8a07af10-6ed8-4c13-bd58-176b5110a9d4
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
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.
Description
DebugLoggerincontainer-loaderis exported only to serve as a namespace for a single static method,mixinDebugLogger. Nothing can actually use the class as a type: it is not re-exported from the package index, appears in no API report, and its constructor is private. The one instance is created inside the mixin and handed tocreateMultiSinkLoggeras anITelemetryBaseLogger, and the mixin returns aTelemetryLoggerExt.This exports the mixin directly as a free function and lets the class become module private. The private constructor is no longer needed to prevent construction from outside the module, so it becomes a normal one. The two in-package call sites (
loader.ts,createAndLoadContainerUtils.ts) now importmixinDebugLoggerinstead ofDebugLogger.No public API change, so no API report updates and no changeset.
Suggested by Jason Hartman (@jason-ha) in review feedback on #28044, split out here to keep that PR focused on telemetry plumbing.
Reviewer Guidance
The review process is outlined in the pull request guidelines.
no-use-before-definestays satisfied. Happy to flip the order if the export-first convention is preferred.