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(install): automatically migrate package-lock.json to bun.lockb #6352

Merged
merged 43 commits into from
Oct 11, 2023

Conversation

paperdave
Copy link
Collaborator

@paperdave paperdave commented Oct 7, 2023

What does this PR do?

If you do not have a bun.lockb, but a package-lock.json exists, this PR reads that data and generates a bun lockfile in memory, and uses that for the install.

I think the migration can be made faster, but people will ideally only run this once per project, so it is fine.

image

If a yarn.lock or pnpm-lock.yaml exists, a warning is printed

image

All of the above only happens if no bun.lockb is present. so this has no perf impact on existing projects.

You can also run bun pm migrate to run just the migrator code path, useful for testing.

There are some situations where this migration is not 100% accurate to what npm installs, which is due to bugs and limitations/optimizations of Bun's package hoisting logic.

var root = Lockfile.Package{};
var needs_new_lockfile = load_lockfile_result != .ok or (load_lockfile_result.ok.buffers.dependencies.items.len == 0 and manager.package_json_updates.len > 0);
var needs_new_lockfile = load_lockfile_result != .ok or load_lockfile_result.ok.buffers.dependencies.items.len == 0 and manager.package_json_updates.len > 0;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is this right

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

probably not. lol

///
/// By default, the underlying buffer is filled with "invalid_id" to indicate this package ID
/// was not resolved
resolutions: PackageIDSlice = .{},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

lets not move the order of these fields

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

damn i didnt realize i did that

src/install/migration.zig Outdated Show resolved Hide resolved
@paperdave paperdave marked this pull request as ready for review October 10, 2023 08:26
@paperdave
Copy link
Collaborator Author

can u add a test case for bun add

@paperdave
Copy link
Collaborator Author

cool

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2023

prettier errors have been resolved. Thank you.

#c58e8f246d1a21ff9a5691e265f35173a6c83c81

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2023

@paperdave 9 files with test failures on bun-darwin-aarch64:

  • test/bundler/esbuild/default.test.ts
  • test/cli/install/bun-install.test.ts
  • test/integration/next/default-pages-dir/test/dev-server.test.ts
  • test/js/bun/test/jest-extended.test.js
  • test/js/bun/test/test-test.test.ts
  • test/js/bun/util/filesink.test.ts
  • test/js/node/watch/fs.watch.test.ts
  • test/js/third_party/postgres/postgres.test.ts
  • test/js/third_party/prisma/prisma.test.ts

View test output

#c58e8f246d1a21ff9a5691e265f35173a6c83c81

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2023

@paperdave 6 files with test failures on linux-x64:

  • test/cli/install/bun-install.test.ts
  • test/cli/install/bun-update.test.ts
  • test/js/bun/test/jest-extended.test.js
  • test/js/node/watch/fs.watch.test.ts
  • test/js/third_party/postgres/postgres.test.ts
  • test/js/third_party/prisma/prisma.test.ts

View test output

#c58e8f246d1a21ff9a5691e265f35173a6c83c81

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2023

@paperdave 6 files with test failures on linux-x64-baseline:

  • test/bundler/esbuild/splitting.test.ts
  • test/cli/install/bun-install.test.ts
  • test/js/bun/test/jest-extended.test.js
  • test/js/node/watch/fs.watch.test.ts
  • test/js/third_party/postgres/postgres.test.ts
  • test/js/third_party/prisma/prisma.test.ts

View test output

#c58e8f246d1a21ff9a5691e265f35173a6c83c81

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2023

@paperdave 9 files with test failures on bun-darwin-x64-baseline:

  • test/cli/install/bun-install.test.ts
  • test/js/bun/spawn/spawn-streaming-stdout.test.ts
  • test/js/bun/spawn/spawn.test.ts
  • test/js/bun/test/jest-extended.test.js
  • test/js/node/fs/fs.test.ts
  • test/js/node/watch/fs.watchFile.test.ts
  • test/js/third_party/postgres/postgres.test.ts
  • test/js/third_party/prisma/prisma.test.ts
  • test/js/third_party/webpack/webpack.test.ts

View test output

#c58e8f246d1a21ff9a5691e265f35173a6c83c81

@Jarred-Sumner Jarred-Sumner merged commit 1bf28e0 into main Oct 11, 2023
15 of 20 checks passed
@Jarred-Sumner Jarred-Sumner deleted the dave/npm-migrate branch October 11, 2023 09:27
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.

None yet

2 participants