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.

Sign up for GitHub

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

Jump to bottom

fix(core): prevent max buffer issues with bunlock files #25985

Merged
merged 1 commit into from
May 27, 2024
Merged

fix(core): prevent max buffer issues with bunlock files #25985

merged 1 commit into from
May 27, 2024

Conversation

Jordan-Hall
Copy link
Contributor

@Jordan-Hall Jordan-Hall commented May 23, 2024 •
edited
Loading

Fixes #25978 #25965

@vercel Vercel
Copy link

vercel bot commented May 23, 2024 •
edited
Loading

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

1 Ignored Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview May 27, 2024 2:30pm

@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented May 23, 2024 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 3c087fc. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

🟥 Failed Commands
nx affected --targets=e2e,e2e-ci --base=aa2519ff62a79790d31f695cf0da5ef0612b0145 --head=3c087fc05ad95b9e841c0d5cf710ab74bf023d2f --parallel=1
✅ Successfully ran 5 targets

Sent with 💌 from NxCloud.

@Jordan-Hall Jordan-Hall force-pushed the fix/25978-max-buffer-limit-issue branch 2 times, most recently from 240f256 to 6a94550 Compare May 23, 2024 21:22
@Jordan-Hall Jordan-Hall marked this pull request as ready for review May 23, 2024 22:04
@Jordan-Hall Jordan-Hall requested a review from a team as a code owner May 23, 2024 22:04
@Jordan-Hall Jordan-Hall force-pushed the fix/25978-max-buffer-limit-issue branch from 6a94550 to 0439554 Compare May 23, 2024 22:42
@Jordan-Hall Jordan-Hall force-pushed the fix/25978-max-buffer-limit-issue branch from 0439554 to 7379155 Compare May 26, 2024 04:25
Jordan-Hall
Jordan-Hall commented May 26, 2024
View reviewed changes
packages/nx/src/plugins/js/index.ts
@@ -98,7 +100,9 @@ export const createDependencies: CreateDependencies = (
const lockFileContents =
packageManager !== 'bun'
? readFileSync(lockFilePath).toString()
: execSync(`bun ${lockFilePath}`).toString();
: execSync(`bun ${lockFilePath}`, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested using the following code snip https://gist.github.com/Jordan-Hall/fb5f5d1e09ebc5d90c67cf1d745e05c8.

This worked using NX own package.json and bun.lockb file. Happy to add test but not sure where

@Jordan-Hall Jordan-Hall mentioned this pull request May 26, 2024
Merged
@Jordan-Hall Jordan-Hall force-pushed the fix/25978-max-buffer-limit-issue branch from 7379155 to 3d66b14 Compare May 26, 2024 10:25
@Jordan-Hall
Copy link
Contributor Author

@FrozenPandaz I'm not sure why CI failing on this one. Not touched that section :/

@benpsnyder
Copy link
Contributor

@FrozenPandaz I'm not sure why CI failing on this one. Not touched that section :/

@Jordan-Hall maybe try rebasing and formatting?
@FrozenPandaz if you can get this one in too I'll be able to test all our repos thoroughly tonight

@Jordan-Hall Jordan-Hall force-pushed the fix/25978-max-buffer-limit-issue branch from 3d66b14 to 3c087fc Compare May 27, 2024 14:27
@Jordan-Hall
Copy link
Contributor Author

@FrozenPandaz I'm not sure why CI failing on this one. Not touched that section :/

@Jordan-Hall maybe try rebasing and formatting? @FrozenPandaz if you can get this one in too I'll be able to test all our repos thoroughly tonight

I've rebased again its nothing to do with linting from looks of it but cypress legacy e2e test... If you can test now without this fix. It works when using bun runtime bunx --bun nx.... The max buffer is a node execSync issue and not a bun execSync issue

@jaysoo
Copy link
Member

jaysoo commented May 27, 2024

@FrozenPandaz I'm not sure why CI failing on this one. Not touched that section :/

@Jordan-Hall maybe try rebasing and formatting? @FrozenPandaz if you can get this one in too I'll be able to test all our repos thoroughly tonight

I've rebased again its nothing to do with linting from looks of it but cypress legacy e2e test... If you can test now without this fix. It works when using bun runtime bunx --bun nx.... The max buffer is a node execSync issue and not a bun execSync issue

Strange indeed. We'll take a look.

@Jordan-Hall
Copy link
Contributor Author

Thank you. I'm having a nightmare with CI recently :(

@jaysoo
Copy link
Member

jaysoo commented May 27, 2024

Thank you. I'm having a nightmare with CI recently :(

I know, we're investigating CI issues for forks. It has regressed since we atomized our tests.

@FrozenPandaz
Copy link
Collaborator

FrozenPandaz commented May 27, 2024 •
edited
Loading

The failing test is unrelated. Thank you for your contribution! LGTM 🎉

@FrozenPandaz FrozenPandaz merged commit ed1f7a6 into nrwl:master May 27, 2024
5 of 7 checks passed
ndcunningham pushed a commit that referenced this pull request May 28, 2024
@Jordan-Hall @ndcunningham
fix(core): prevent max buffer issues with bunlock files (#25985)
908fb7e 
Fixes #25978 #25965
FrozenPandaz pushed a commit that referenced this pull request May 28, 2024
@Jordan-Hall @FrozenPandaz
fix(core): prevent max buffer issues with bunlock files (#25985)
edd5d36 
Fixes #25978 #25965

(cherry picked from commit ed1f7a6)
@cskiwi
Copy link

cskiwi commented Jun 1, 2024 •
edited
Loading

The error is still happening for me, (nx 19.1.1 and bun 1.1.10)

@Jordan-Hall
Copy link
Contributor Author

The error is still happening for me, (nx 19.1.1 and bun 1.1.10)

The fix was is in 19.2 I believe

@cskiwi
Copy link

cskiwi commented Jun 1, 2024 •
edited
Loading

The error is still happening for me, (nx 19.1.1 and bun 1.1.10)

The fix was is in 19.2 I believe

From the changelog it should have been in 19.1.1? https://github.com/nrwl/nx/releases/tag/19.1.1
Ah, it's in both changelogs.

However I updated to 19.2.0-beta.5 and the issue is there.
Update: correction, it seems to be another error now. investigating...

@Jordan-Hall
Copy link
Contributor Author

Jordan-Hall commented Jun 1, 2024 •
edited
Loading

Please go to the issue that's open and share errors message please. I think it's a different error

#26053

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 7, 2024

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jaysoo jaysoo jaysoo approved these changes

@FrozenPandaz FrozenPandaz FrozenPandaz approved these changes

Assignees

@jaysoo jaysoo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When using Bun and building in containers, respect the yarn.lock file
5 participants
@Jordan-Hall @benpsnyder @jaysoo @FrozenPandaz @cskiwi