-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: Vercel ai
ESM patching
#16152
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
fix: Vercel ai
ESM patching
#16152
Conversation
size-limit report 📦
|
513e696
to
f9b14f4
Compare
f9b14f4
to
d48395d
Compare
ai
import-in-the-middle
patchingai
ESM patching
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.
Thanks for fixing this and for the great context around the root cause!
hmm look slike CI is stuck for some reason 🤔 @timfish mind rebasing to see if this fixes CI? thx! |
Not sure how this made it onto master. Maybe a caching issue?
|
hmm maybe 🤔 we did enable import sorting again via #16134, so this is likely related |
No idea why remix-hydrogen e2e test is failing 🤷♂️ |
hmm this looks unrelated indeed. Will take a look update: yeah, it's also failing in other PRs. We'll fix this |
CI should be unblocked -- @timfish please rebase once more, thx! |
With ESM patching you need to retain the original module and just overwrite the exports you want to wrap. This usually works for CJS too but unfortunately, the CJS exports of
ai
only have getters and no setters so this route is not possible.This PR changes the patching so that it works slightly differently for ESM and CJS. The original code that outputs a newly created object is retained for CJS but for ESM we use the preferred route of replacing the required exports.
To detect whether the module we're patching is an ES module we check
Object.prototype.toString.call(moduleExports) === '[object Module]'
which is documented here.This PR also adds an ESM test for
ai
.