Skip to content

Conversation

@mattieruth
Copy link
Contributor

@mattieruth mattieruth commented Dec 19, 2025

@jptaylor @Regaddi -- Per #114 -- Is there any reason NOT to do this?

Copy link

Copilot AI left a comment

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 transitions the @pipecat-ai/client-js package to official ES module support by adding "type": "module" to package.json and removing the separate "module" field. This simplifies the package configuration by committing fully to ES modules rather than providing dual CommonJS/ESM outputs.

  • Adds "type": "module" to mark the package as using ES modules
  • Removes the "module" field that previously pointed to dist/index.module.js
  • Retains only the "main" field pointing to dist/index.js for the single ESM output

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

"name": "@pipecat-ai/client-js",
"version": "1.5.0",
"license": "BSD-2-Clause",
"type": "module",
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

Adding "type": "module" to package.json requires Jest/ts-jest ESM configuration. The current Jest configuration uses "preset": "ts-jest" without ESM-specific settings. When "type": "module" is set, ts-jest needs to use the "ts-jest/presets/default-esm" preset instead, and additional configuration is needed:

  1. Change the preset to "ts-jest/presets/default-esm"
  2. Add "extensionsToTreatAsEsm": [".ts"]
  3. Add "moduleNameMapper" with "^(\.{1,2}/.*)\.js$": "$1" to handle .js imports from .ts files

Without these changes, tests may fail when running with the ESM module type.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤨 -- and yet, they ran and passed without these changes. of course, we don't have many so... 🤔

"type": "module",
"main": "dist/index.js",
"module": "dist/index.module.js",
"types": "dist/index.d.ts",
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

Removing the "module" field while adding "type": "module" changes the package's export strategy. Consider adding an "exports" field to provide explicit entry points for modern Node.js and bundlers. This is the current best practice for ES modules:

"exports": {
  ".": {
    "types": "./dist/index.d.ts",
    "default": "./dist/index.js"
  }
}

This provides better control over how the package is consumed and prevents deep imports. Note that client-react still uses the dual "main"/"module" approach, so this change creates inconsistency between the two packages in this monorepo.

Suggested change
"types": "dist/index.d.ts",
"types": "dist/index.d.ts",
"exports": {
".": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}
},

Copilot uses AI. Check for mistakes.
@mattieruth mattieruth marked this pull request as draft December 19, 2025 19:49
@mattieruth
Copy link
Contributor Author

blech. reading up more on this. The format we've gone with seems to be the preferred way by parcel -- it mentions that you can use "type": "module" but as like an afterthought without saying if anything else needs to be changed or added.

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.

2 participants