Skip to content

remove outdated deps#229

Merged
owlstronaut merged 2 commits intomainfrom
remove-outdated-deps
Feb 26, 2026
Merged

remove outdated deps#229
owlstronaut merged 2 commits intomainfrom
remove-outdated-deps

Conversation

@owlstronaut
Copy link
Copy Markdown
Contributor

  • feat: use node:crypto instead of imurmurhash
  • deps: remove imurmurhash

@owlstronaut owlstronaut requested a review from a team as a code owner February 25, 2026 23:11
@wraithgar
Copy link
Copy Markdown
Member

iirc imurmurhash was picked for performance reasons

This version works significantly faster than the non-incremental version if you need to hash many small strings into a single hash

We may want to do a quick benchmark to make sure we're not getting wildly out of frame here.

@owlstronaut
Copy link
Copy Markdown
Contributor Author

That sounds like a fun experiment :)

@owlstronaut owlstronaut marked this pull request as draft February 26, 2026 17:30
@owlstronaut
Copy link
Copy Markdown
Contributor Author

@wraithgar

I ran some benchmarks on this. imurmurhash is faster, but the difference is negligible:

  • A single getTmpname hash takes ~0.7 µs with crypto.sha1 vs ~0.14 µs with imurmurhash
  • A single fs.writeFileSync takes ~500 µs so the hash is 0.1% of the actual write operation either way
  • In the npm CLI, write-file-atomic is only used transitively through bin-links (in fix-bin.js), and only fires when a bin script has Windows \r\n line endings, a rare edge case.

For unique-slug (which I'd also like to make this change in), cacache calls it once per content write. Even on a large install with 1,000 packages, the total
difference across the entire operation would be ~0.5 ms, completely lost in the noise of network and disk I/O.

I think using this hash originally was a hobbyist optimization

@owlstronaut owlstronaut marked this pull request as ready for review February 26, 2026 18:35
Copy link
Copy Markdown
Member

@wraithgar wraithgar left a comment

Choose a reason for hiding this comment

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

nit: this is a fix not a feat imvhmo

@owlstronaut owlstronaut merged commit 727e92c into main Feb 26, 2026
14 checks passed
@owlstronaut owlstronaut deleted the remove-outdated-deps branch February 26, 2026 21:44
@github-actions github-actions bot mentioned this pull request Feb 26, 2026
owlstronaut pushed a commit that referenced this pull request Feb 26, 2026
🤖 I have created a release *beep* *boop*
---


##
[7.0.1](v7.0.0...v7.0.1)
(2026-02-26)
### Bug Fixes
*
[`da246ef`](da246ef)
[#229](#229) use
node:crypto instead of imurmurhash (@owlstronaut)
### Dependencies
*
[`727e92c`](727e92c)
[#229](#229) remove
imurmurhash
### Chores
*
[`4785863`](4785863)
[#221](#221) bump
@npmcli/eslint-config from 5.1.0 to 6.0.0 (#221) (@dependabot[bot])
*
[`0c819a3`](0c819a3)
[#223](#223) bump
@npmcli/template-oss from 4.28.0 to 4.28.1 (#223) (@dependabot[bot],
@npm-cli-bot)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

2 participants