search

openai / openai-node

The official Node.js / Typescript library for the OpenAI API
https://www.npmjs.com/package/openai
Apache License 2.0
7.6k stars 799 forks source link

Stop using polyfills for Node.js #951

Closed mcollina closed 1 month ago

mcollina commented 1 month ago

Confirm this is a feature request for the Node library and not the underlying OpenAI API.

Describe the feature or improvement you're requesting

My understanding is that this module use polyfills for Node.js. There is no need for doing so given you support Node.js v18+.

This causes compatibility issues with the rest of the ecosystem.

Ref https://github.com/fastify/fastify/issues/5584

Additional context

No response

RobertCraigie commented 1 month ago

Hi @mcollina, see #392 for some context. We were originally thinking about depending on undici but it looks like we'll be able to depend entirely on builtins.

Unfortunately that change won't land particularly quickly, is there anything we could to do in the meantime to be compatible with fastify?

mcollina commented 1 month ago

The problem is not actually in fastify. We call Readable.fromWeb().

I think providing a default utility to get a Node.js stream instead would remove the problem.

Ethan-Arrowood commented 1 month ago

👋 Hi Matteo!

Hopefully removing the polyfill will fix that user's issue. As @RobertCraigie mentioned, more work is happening behind the scenes to align the SDK with more Node.js built-ins and align on web standard APIs so the SDK is interoperable between any JS Runtime environment.

Keep an eye out for future updates 🚀

RobertCraigie commented 1 month ago

@mcollina this will be fixed in the next release :)

https://github.com/openai/openai-node/pull/949

RobertCraigie commented 1 month ago

This was fixed in v4.53.1