-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(node): Assign default export of openai
to the instrumented fn
#17320
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
openai
to the instrumented fnopenai
to the instrumented fn
size-limit report 📦
|
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.
can we maybe also add a test covering this? (could also be in a follow up)
try { | ||
exports.default = WrappedOpenAI; | ||
} catch (error) { | ||
Object.defineProperty(exports, 'default', { |
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.
could we add a comment here explaining what we do here and why? When would this fail, and would the catch block work then? Can/should we just always do the catch block here?)
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.
on it
#17320) Both of: - import OpenAI from `openai` - import { OpenAI } from`openai` Should point to the instrumented wrapper function.
Both of: - import OpenAI from `openai` - import { OpenAI } from`openai` Should point to the instrumented wrapper function. Backport of #17320.
Both of:
openai
openai
Should point to the instrumented wrapper function.