Skip to content

Fix dual ESM/CJS output, Azure Monitor disable flag - #30

Merged
Hector Hernandez (hectorhdzg) merged 3 commits into
microsoft:mainfrom
hectorhdzg:hectorhdzg/distro-fixes
Apr 22, 2026
Merged

Hector Hernandez (hectorhdzg) merged 3 commits into
microsoft:mainfrom
hectorhdzg:hectorhdzg/distro-fixes

Conversation

@hectorhdzg

Copy link
Copy Markdown
Member
  • Add post-build CJS fixups (scripts/fixup-cjs.cjs):
    • dist/commonjs/package.json with {type:commonjs} so Node respects CJS
    • Copy module-cjs.cts polyfill over module.ts (import.meta.url is ESM-only)
    • Fix Azure Functions default-import in CJS output (__importDefault double-wrap)
  • Fix Azure Functions import for ESM: use default-import + destructure because the package is a webpack bundle and Node ESM cannot extract named exports
  • Add enabled flag to AzureMonitorOpenTelemetryOptions (default true) so consumers can run the distro without Azure Monitor
  • Make all Azure Monitor handlers conditional in distro.ts
  • Add missing re-exports to index.ts: Modality, InvocationRole, A365_MESSAGE_SCHEMA_VERSION, TextPart, ToolCallRequestPart, ToolCallResponsePart, ReasoningPart

- Add post-build CJS fixups (scripts/fixup-cjs.cjs):
  * dist/commonjs/package.json with {type:commonjs} so Node respects CJS
  * Copy module-cjs.cts polyfill over module.ts (import.meta.url is ESM-only)
  * Fix Azure Functions default-import in CJS output (__importDefault double-wrap)
- Fix Azure Functions import for ESM: use default-import + destructure
  because the package is a webpack bundle and Node ESM cannot extract named exports
- Add enabled flag to AzureMonitorOpenTelemetryOptions (default true)
  so consumers can run the distro without Azure Monitor
- Make all Azure Monitor handlers conditional in distro.ts
- Add missing re-exports to index.ts: Modality, InvocationRole,
  A365_MESSAGE_SCHEMA_VERSION, TextPart, ToolCallRequestPart,
  ToolCallResponsePart, ReasoningPart

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the package’s dual ESM/CJS distribution behavior (especially around Azure Functions + CJS semantics) and introduces an azureMonitor.enabled flag so consumers can run the distro without Azure Monitor while still using other components.

Changes:

  • Add a post-build step to patch dist/commonjs so Node treats it as CJS and to fix Azure Functions import output.
  • Add azureMonitor.enabled?: boolean (default true) and conditionally initialize Azure Monitor components/handlers in useMicrosoftOpenTelemetry().
  • Add missing public re-exports from the A365 surface and adjust tests to explicitly disable Azure Functions instrumentation where needed.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test/internal/unit/traces/traceHandler.test.ts Explicitly disables azureFunctions instrumentation in relevant test configs.
src/types.ts Adds AzureMonitorOpenTelemetryOptions.enabled?: boolean to support disabling Azure Monitor.
src/index.ts Re-exports additional A365 enums/constants/types from the top-level entrypoint.
src/distro/distro.ts Makes Azure Monitor setup conditional and avoids registering its handlers when disabled.
src/azureMonitor/traces/handler.ts Switches Azure Functions instrumentation import style to work in Node ESM for a bundled CJS module.
scripts/fixup-cjs.cjs New post-build script to correct CJS output behavior in dist/commonjs.
package.json Wires fixup-cjs into the build script.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/fixup-cjs.cjs Outdated
Comment thread src/distro/distro.ts
Comment thread src/distro/distro.ts
Comment thread src/distro/distro.ts
Comment thread scripts/fixup-cjs.cjs
@hectorhdzg
Hector Hernandez (hectorhdzg) merged commit 2b461db into microsoft:main Apr 22, 2026
5 checks passed
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.

3 participants