-
Notifications
You must be signed in to change notification settings - Fork 170
chore: Add commonjs builds to openinference-genai #2505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@arizeai/openinference-core
@arizeai/openinference-genai
@arizeai/openinference-instrumentation-anthropic
@arizeai/openinference-instrumentation-bedrock
@arizeai/openinference-instrumentation-bedrock-agent-runtime
@arizeai/openinference-instrumentation-beeai
@arizeai/openinference-instrumentation-langchain
@arizeai/openinference-instrumentation-langchain-v0
@arizeai/openinference-instrumentation-mcp
@arizeai/openinference-instrumentation-openai
@arizeai/openinference-mastra
@arizeai/openinference-semantic-conventions
@arizeai/openinference-vercel
commit: |
| "types": "dist/esm/index.d.ts", | ||
| "main": "dist/src/index.js", | ||
| "module": "dist/esm/index.js", | ||
| "types": "dist/src/index.d.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Main and types fields point to non-existent paths
The main and types fields point to dist/src/index.js and dist/src/index.d.ts, but the build script only generates output to dist/commonjs/ and dist/esm/ directories. The tsconfig.commonjs.json has outDir: "dist/commonjs" with rootDir: "src", so files compile to dist/commonjs/index.js. The dist/src/ directory will never be created, causing Node.js to fail to resolve the package when these fallback fields are used.
| "./types": { | ||
| "types": "./dist/esm/types.d.ts" | ||
| "types": "./dist/esm/types.d.ts", | ||
| "require": "./dist/commonjs/types.d.ts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Require condition points to type definition file instead of JavaScript
The require condition for the ./types export points to ./dist/commonjs/types.d.ts, which is a TypeScript declaration file, not executable JavaScript. When CommonJS consumers use require("@arizeai/openinference-genai/types"), Node.js will attempt to load the .d.ts file as JavaScript and fail. The require condition needs to point to a .js file (e.g., ./dist/commonjs/types.js).
Import and require appear to both work in my sample repo. require does not get type information though, users will need to write esm and compile/execute to commonjs for typings (which is common).
In addition will need to bump https://github.com/mastra-ai/mastra/blob/d7509c2b59f3772a91c8f32c37b9e93b64c7da7d/observability/arize/package.json#L34
Note
Add CommonJS build and dual ESM/CJS exports for
@arizeai/openinference-genai, updating build scripts and TS configs, with a patch changeset.tsconfig.commonjs.json; updatebuildto compile both CJS and ESM.package.jsonwith"require"entries for.,./attributes,./utils, and./types; set"main"todist/src/index.js,"module"todist/esm/index.js, and update"types".module: "preserve",moduleResolution: "bundler", setnoEmitandtsBuildInfoFilepaths.@arizeai/openinference-genai.Written by Cursor Bugbot for commit b81b766. This will update automatically on new commits. Configure here.