-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
chore: extract rsc to separate package #5542
Conversation
package.json
Outdated
"type-check": "tsc --build", | ||
"type-check:full": "tsc --build tsconfig.with-examples.json", |
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.
which one will run in CI? important that we keep full checks there
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.
i've updated it to run type-check:full
in CI. I made type-checking faster in the IDE that uses the default tsconfig (including examples in it makes it way slower)
@@ -9,6 +9,7 @@ | |||
"forceConsistentCasingInFileNames": true, | |||
"inlineSources": false, | |||
"isolatedModules": true, | |||
"module": "ESNext", |
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.
is there a risk that this breaks common js usage of the ai sdk?
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.
needed for the rsc move?
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.
i think this was required for one of the builds to work when moving to the react package cause it's got a different config, so i added it to the base since it's best practise to set the module to ESNext when you're targeting a bundler, to make sure it can do dead code removal correctly. But I can also set it in the react tsconfig itself if we want to
This just affects how typescript compiles to JS - but we pass it through tsup so it converted at later stage back to commonjs
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.
it should have no effect - but i'm going to thoroughly inspect the compiled package to ensure this next
e6d9f7f
to
a51b6c6
Compare
a51b6c6
to
760ebfc
Compare
760ebfc
to
1d863f8
Compare
8516063
to
3dae73a
Compare
@@ -1,5 +1,5 @@ | |||
{ | |||
"extends": "./node_modules/@vercel/ai-tsconfig/react-library.json", | |||
"extends": "./node_modules/@vercel/ai-tsconfig/base.json", | |||
"compilerOptions": { |
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 the dom
, dom.iterable
libs be removed below? (worth a try)
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.
3dae73a
to
9d034ea
Compare
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.
Please add a changeset for ai
and @ai-sdk/rsc
This removes & moves over the
ai/rsc
bundle to instead be in@ai-sdk/react/rsc
. And also removes the dependency on react from theai
packageai/internal
bundle with the newly needed exports (does duplication for now, separate PR will fix that)ai
packagersc
package (✅ confirmed to all be running in new package)