-
Notifications
You must be signed in to change notification settings - Fork 256
feat: add dual esm + cjs build with shim guard, updated exports, and lambda support #333
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
Conversation
🦋 Changeset detectedLatest commit: 80cdb88 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Thanks for sending this. I didn't have time for checking this repo today, but will check this one and #258 to resolve this issue soon. |
Thanks for sending this. I am now checking this branch on my local machine. First thing I noticed is these warnings introduced in this PR. Can you take a look?
Also, when I go through these steps here,
So, something needs to be addressed. I will look into alternative solutions for this too. |
My #346 should resolve this issue; if you have any comments, please feel free to write in. |
273e3f0
to
80cdb88
Compare
Thanks for flagging! I reproduced the warnings/errors locally. I've updated the root build script to call Also fixed CJS build type errors in Both ESM and CJS builds now run clean, and verdaccio + integration tests pass here. Happy to dig in further if you see issues. BTW I can to rebase #333 on top of #346 based on your guidance on that thread. |
Closing in favor of #346 |
Hey @seratch, here’s the PR. It’s been tested locally and directly on AWS Lambda. A
lambda-local
test is included in the PR, and I’ve documented the steps for building and testing on Lambda itself in this gistDual build – every public package now emits:
dist/
→ ES 2022 (unchanged, ESM users are unaffected – import keeps loading the original ES modules.)dist-cjs/
→ CommonJS (module: "commonjs"
)Exports wiring –
main
,exports.import
/require
, andtypes
point at the correct build for every sub-path (./realtime
,./utils
,_shims
…).Shim guard –
loadEnv()
now uses:so
require()
no longer crashes in CJS runtimes.Realtime CJS build –
@openai/agents-realtime
gets its owndist-cjs/
output and is fully exported.Integration tests:
integration-tests/esm-import
– proves dynamicimport()
on Node 22integration-tests/lambda-local
– spins uplambda-local
and asserts a 200 response from a CommonJS handler that uses bothAgent
andOpenAIRealtimeWebSocket
.Changeset – bumps all affected packages with a patch release.
Some thoughts:
We can run the realtime transport inside the same CJS process that handles Twilio events, enabling “live call” use‑cases without extra shims. Also, we can just drop the SDK into Twilio Functions or a Flex Plugin without converting the whole workspace to ESM or adding a bundler step. (Overall, better DX for TS code bases that remain on CJS.)
Of course, AWS Lambda, Azure Functions, and Google Cloud Functions all support native ESM, but every one of them still defaults to CJS for a typical “zip‑and‑upload” or framework‑generated project.
Because the Agents SDK now ships a first‑class CJS build, we can drop it into those default workflows without extra flags or custom runtimes. (If a team is already running an explicit ESM handler e.g. .mjs file or "type":"module"then the existing ESM build continues to work.)
Happy to address any feedback, please feel free to convert this to a draft for further review if needed. Thanks!
Resolves #245 and resolves #213