Skip to content
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

When used in Node (eg with Next.JS), unnecessarily overwrites process object #44

Closed
rattrayalex opened this issue Aug 21, 2023 · 3 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@rattrayalex
Copy link

rattrayalex commented Aug 21, 2023

Most polyfills don't mutate the object when it isn't needed, but this one seems to do so.

Context in this thread: openai/openai-node#226

Observed behavior when using this library in Node 18 with NextJS:

Object.prototype.toString.call(process) === '[object Object]'

Expected behavior:

Object.prototype.toString.call(process) === '[object process]'

The observed behavior is correct in a non-node context, like the browser; please don't change it for that. However, if used in Node, I would expect this library to not mutate process.

@afonsojramos
Copy link
Contributor

I believe this is user error though, because one should not use polyfills if you're running on a Node environment, right?

@rattrayalex
Copy link
Author

The code that calls this library may not know whether or not it's being used in a Node context.

For example, they may be compiling the same slug to be deployed to both Node and non-Node contexts.

In my experience, it's common/expected/desired behavior for importing a polyfill to be a no-op when it's not needed (eg, importing this library when you're already in Node).

Does that make sense?

@afonsojramos
Copy link
Contributor

I suppose so, yeah!

Richienb added a commit that referenced this issue May 24, 2024
…` modules by default; rename `includeAliases` to `additionalAliases`, and allow ignoring the defaults with `onlyAliases`

Fixes #40, #52, #18, #44

Signed-off-by: Richie Bendall <richiebendall@gmail.com>
Richienb added a commit that referenced this issue May 24, 2024
…` modules by default; rename `includeAliases` to `additionalAliases`, and allow ignoring the defaults with `onlyAliases`

Fixes #40, fixes #52, fixes #18, fixes #44

Signed-off-by: Richie Bendall <richiebendall@gmail.com>
Richienb added a commit that referenced this issue May 24, 2024
…` modules by default; rename `includeAliases` to `additionalAliases`, and allow ignoring the defaults with `onlyAliases`

Fixes #40, fixes #52, fixes #18, fixes #44, fixes #20

Signed-off-by: Richie Bendall <richiebendall@gmail.com>
Richienb added a commit that referenced this issue May 24, 2024
* feat: add `fs` fallback

* Update readme.md

* Stop polyfilling `console`, 'domain', `process`, and internal `stream` modules by default; rename `includeAliases` to `additionalAliases`, and allow ignoring the defaults with `onlyAliases`

Fixes #40, fixes #52, fixes #18, fixes #44

Signed-off-by: Richie Bendall <richiebendall@gmail.com>

* Add as default

Signed-off-by: Richie Bendall <richiebendall@gmail.com>

* Update readme.md

---------

Signed-off-by: Richie Bendall <richiebendall@gmail.com>
Co-authored-by: Richie Bendall <richiebendall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants