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

NextJS Codegen #1346

Open
wants to merge 12 commits into
base: canary
Choose a base branch
from
Open

NextJS Codegen #1346

wants to merge 12 commits into from

Conversation

seawatts
Copy link
Contributor

This adds native integration for next.js code generation.

Copy link

vercel bot commented Jan 17, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
baml ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 7, 2025 0:15am

Copy link
Contributor

ellipsis-dev bot commented Jan 17, 2025

⚠️ This PR is too big for Ellipsis, but support for larger PRs is coming soon. If you want us to prioritize this feature, let us know at [email protected]


Generated with ❤️ by ellipsis.dev

@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from 3b09c1e to 922ac1c Compare January 17, 2025 19:04
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from 2f0579f to 43eb31f Compare January 30, 2025 00:50
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from 210977c to 0219ef1 Compare January 30, 2025 21:58
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from 04e9f20 to f758959 Compare January 30, 2025 22:05
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from f758959 to c75e1a0 Compare January 30, 2025 22:22
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from c75e1a0 to ea9b375 Compare January 30, 2025 22:25
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from ea9b375 to 743bf9c Compare February 3, 2025 22:33
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from 43628df to e4e9830 Compare February 5, 2025 16:46
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from e4e9830 to d28aa93 Compare February 5, 2025 16:47
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch from d28aa93 to f2dbc25 Compare February 5, 2025 17:02
@seawatts seawatts force-pushed the seawatts/next-js-codegen branch 2 times, most recently from f9b90cf to ea6f658 Compare February 5, 2025 23:31
Copy link
Contributor

@imalsogreg imalsogreg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple comments, none of them critical.

Looks pretty great!

Note - I haven't run integ tests myself.

I did run pnpm generate but didn't carefully examine the results.

Happy to do the above things soon, just wanted to give my initial 👍 in case you are trying to merge quickly.

engine/language_client_codegen/src/typescript/mod.rs Outdated Show resolved Hide resolved
engine/language_client_typescript/artifacts/native.d.ts Outdated Show resolved Hide resolved
engine/language_client_typescript/stream.js Outdated Show resolved Hide resolved
fern/.cursorrules Show resolved Hide resolved
fern/docs.yml Outdated Show resolved Hide resolved
integ-tests/README.md Show resolved Hide resolved
Copy link
Collaborator

@sxlijin sxlijin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reading docs now

Comment on lines +2 to +3
import type { Checked, Check } from "./types"
import type { {% for t in types %} {{ t }}{% if !loop.last %}, {% endif %}{% endfor %} } from "./types"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a name collision, that users can't use "Check" for their own type name - or is that a decision we already made? (Same also applies to "Image" and "Audio", but I think that one is less egregious.)

cc @imalsogreg

@@ -5,6 +5,6 @@ if (require.main === module) {
process.env.BAML_LOG = 'info'
}

const baml = require('./native')
const baml = require('./artifacts/native')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we split this change out of this PR?

I tried to do something along these lines a while back and screwed up the TS release in a hard-to-understand way. We can try another PR to push this forward but I'm uncomfortable doing that in this PR.

Comment on lines +41 to +42
npm install @boundaryml/baml
npm install @boundaryml/baml-nextjs-plugin
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
npm install @boundaryml/baml
npm install @boundaryml/baml-nextjs-plugin
npm install @boundaryml/baml @boundaryml/baml-nextjs-plugin

and I think we can do the same for the others too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants