Skip to content

Conversation

@yjoer
Copy link
Contributor

@yjoer yjoer commented Jul 10, 2025

I replaced node-pre-gyp with the latest version, shrinking the dependency graph from 95 to 56 packages. Since both the regular and coverage test modes now load the loader from source, output discrepancies between them have been eliminated. Additionally, I've added integration tests for both node-pre-gyp and node-gyp-build, using bcrypt versions 5 and 6. The split chunk logic in the test project has also been updated so that each entry point now generates its own vendor chunk.

Closes #194

@yjoer yjoer requested review from Timer and styfle as code owners July 10, 2025 19:11
@socket-security
Copy link

socket-security bot commented Jul 10, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedresolve-from@​3.0.01001008676100
Addedini@​1.3.810010010083100
Addedbcrypt@​5.1.19210010085100
Addedbcrypt@​6.0.010010010085100
Added@​mapbox/​node-pre-gyp@​2.0.09810010085100

View full report

@styfle styfle changed the title build: remove the build step and load the loader from source feat: remove the build step and load the loader from source Jul 10, 2025
main: './main.js',
chunk: './chunk.js',
bcrypt: './packages/bcrypt.js',
bcrypt5: './packages/bcrypt5.js',
Copy link
Member

Choose a reason for hiding this comment

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

Should this change be a separate PR or is there a reason we need it for this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

node-pre-gyp was updated. While unit tests were already in place, I added an integration test to provide additional confidence as the foundation was laid in the previous PR. bcrypt v5 is the version that uses node-pre-gyp. And since bcrypt v5 was added, I have also included bcrypt v6.

Copy link
Member

Choose a reason for hiding this comment

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

Usually I would say this should be a separate PR to prove it was passing before and then the PR to change to mapbox would prove its still passing but I think its fine to leave as is

@styfle styfle merged commit 080ddf8 into vercel:main Jul 10, 2025
9 checks passed
@github-actions
Copy link

🎉 This PR is included in version 1.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@yjoer yjoer deleted the patch-4 branch July 25, 2025 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants