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

feat(turborepo): support Bun #5934

Merged
merged 11 commits into from
Sep 15, 2023
Merged

Conversation

nathanhammond
Copy link
Contributor

@nathanhammond nathanhammond commented Sep 12, 2023

Enable using bun as both a package manager and a runtime.

Closes #4762

Closes TURBO-1324

@vercel
Copy link

vercel bot commented Sep 12, 2023

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

Name Status Preview Comments Updated (UTC)
examples-cra-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Sep 15, 2023 3:10am
examples-vite-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Sep 15, 2023 3:10am
9 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2023 3:10am
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2023 3:10am
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2023 3:10am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2023 3:10am
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2023 3:10am
examples-nonmonorepo ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2023 3:10am
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2023 3:10am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2023 3:10am
turbo-site ⬜️ Ignored (Inspect) Visit Preview Sep 15, 2023 3:10am

Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca

@vercel
Copy link

vercel bot commented Sep 12, 2023

Someone is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@gsoltis gsoltis requested review from tknickman, gsoltis and chris-olszewski and removed request for chris-olszewski September 12, 2023 17:31
let repo_root_path = AbsoluteSystemPathBuf::try_from(repo_root.path())?;

let lockfile_path = repo_root.path().join(LOCKFILE);
File::create(lockfile_path)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking nit: we have .create_with_contents("") so we don't need to pull in std::fs::File.

Copy link
Member

@chris-olszewski chris-olszewski left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. A few questions:

  • Are we expecting users to have print = "yarn" in their in their bunfig.toml in order to get lockfile based info for their repos? If so we should probably have some custom error messaging in the package graph building that suggests adding it when we notice they have a bun.lockb but no yarn.lock that we can read.
  • I'm pretty strongly against claiming we offer any sort of prune support until we can serialize enough info for some version of bun install to work. (Unless I'm reading the docs wrong and users can just bun install --yarn with only a yarn.lock and not need a bun.lockb)

crates/turborepo-lockfiles/src/bun/mod.rs Outdated Show resolved Hide resolved
crates/turborepo-lockfiles/src/bun/mod.rs Outdated Show resolved Hide resolved
crates/turborepo-lockfiles/src/bun/mod.rs Show resolved Hide resolved
crates/turborepo-lockfiles/src/bun/ser.rs Outdated Show resolved Hide resolved
cli/internal/packagemanager/npm.go Show resolved Hide resolved
cli/internal/packagemanager/bun.go Show resolved Hide resolved
@nathanhammond
Copy link
Contributor Author

nathanhammond commented Sep 13, 2023

Are we expecting users to have print = "yarn" in their in their bunfig.toml in order to get lockfile based info for their repos? If so we should probably have some custom error messaging in the package graph building that suggests adding it when we notice they have a bun.lockb but no yarn.lock that we can read.

Nope, bun bun.lockb spits out the yarn lockfile, so I'm going to do it "just in time" every time. I don't want to ask users to have to do anything.

I'm pretty strongly against claiming we offer any sort of prune support until we can serialize enough info for some version of bun install to work. (Unless I'm reading the docs wrong and users can just bun install --yarn with only a yarn.lock and not need a bun.lockb)

I've started a thread in Slack on this to get full team input.

Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca

Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca

Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca

@nathanhammond nathanhammond merged commit 0239f38 into vercel:main Sep 15, 2023
46 checks passed
@juliusmarminge
Copy link

juliusmarminge commented Sep 15, 2023

Thanks for shipping this so fast. Just tried out the canary and noticed some warnings:

  1. Vercel detects the bun lockfile, but then proceeds using yarn, I had to manually set bun install as the install command to fix this:
CleanShot 2023-09-15 at 08 55 01@2x
  1. On Vercel, I get an error/warning about no package manager being set in package.json. I believe you can't put bun in there, at least you get warnings saying that's not a valid option?
CleanShot 2023-09-15 at 08 54 36@2x
  1. Running the suggested codegen command also errors. Might be worth gracefully exiting here.
CleanShot 2023-09-15 at 08 56 37@2x

EDIT: Actually running the canaray version of codegen package works:
CleanShot 2023-09-15 at 08 58 05@2x

But we do get the warning that bun isn't a valid option, can we just ignore that?
CleanShot 2023-09-15 at 08 58 10@2x

@mpotane
Copy link

mpotane commented Sep 15, 2023

same here look like corepack issue nodejs/corepack#307

@nathanhammond
Copy link
Contributor Author

@juliusmarminge Thanks for the quick feedback!

  1. Without doing any investigation, root cause here is likely the "adjusting default settings" step in there. Will investigate what happened. That step is not inside of turbo so your repository and turbo itself are probably fine. A few changes on our side and a redeploy to fix it.
  2. We're intentionally breaking the rules about packageManager. That warning does not introduce bugs and can be safely ignored.
  3. You did correctly identify that the codemod needed to be @canary.

Aside, the opener for the corepack PR is also a Vercel person. We're paying attention to it.

@styfle
Copy link
Member

styfle commented Sep 15, 2023

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.

Support bun (bun.sh)
6 participants