chore: support CJS #883
chore: support CJS #883
Conversation
this PR changes the CI to run two protobuf generators, one for each, plus another for dts. currently untested.
|
packages/javascript/src/index.d.ts
Outdated
| export * from "./gen/livekit_sip_pb.js"; | ||
| export * from "./gen/livekit_webhook_pb.js"; | ||
| export * from "./gen/version.js"; | ||
| export type * from "./gen/dts/livekit_agent_dispatch_pb.d.ts"; |
There was a problem hiding this comment.
hm, don't remember ever seeing this import pattern 🤔
There was a problem hiding this comment.
the LSP didn't complain, but i'll double check to make sure this works
There was a problem hiding this comment.
works great, except the files under cjs need to be renamed to .cjs in order to be required as CommonJS. i don't know if GH Actions comes built in with any rename utility
There was a problem hiding this comment.
ok so i did a bulkrename but now i need to sed through all the files and replace the names as well
There was a problem hiding this comment.
would be nice if we could configure that as part of the protocol es plugin, I think file extension is an option there?
There was a problem hiding this comment.
only able to set js/ts/d.ts on there unfortunately
|
tested working on livekit-server-sdk (which breaks for a different reason, will be handled there, this part is fine) |
packages/javascript/src/index.d.ts
Outdated
| export * from "./gen/livekit_sip_pb.js"; | ||
| export * from "./gen/livekit_webhook_pb.js"; | ||
| export * from "./gen/version.js"; | ||
| export type * from "./gen/dts/livekit_agent_dispatch_pb.d.ts"; |
There was a problem hiding this comment.
would be nice if we could configure that as part of the protocol es plugin, I think file extension is an option there?
|
before merging this, it would be great if we could test and verify the ESM and CJS behaviour with the most common bundlers/frameworks
|
|
tested building downstream with esbuild (agents + tsup) |
this PR changes the CI to run two protobuf generators, one for each, plus another for dts. currently untested.