Skip to content

Commit

Permalink
deps: remove promise-inflight (#223)
Browse files Browse the repository at this point in the history
The use of promise-inflight was originally intended to prevent duplicate
concurrent git ls-remote calls. However, this scenario is rare, and the
performance impact of duplicate calls is minimal. The existing LRU cache
already prevents redundant requests over time.

Removing promise-inflight simplifies the code and removes a dependency
often cited by security linters. This does mean duplicate inflight
requests are now possible, but the expected impact is negligible. Should
we want to make sure they don't happen, we could inline a solution
fairly easily
  • Loading branch information
owlstronaut authored Feb 18, 2025
1 parent 29dfb33 commit 618c70c
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 11 deletions.
16 changes: 5 additions & 11 deletions lib/revs.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
const pinflight = require('promise-inflight')
const spawn = require('./spawn.js')
const { LRUCache } = require('lru-cache')
const linesToRevs = require('./lines-to-revs.js')

const revsCache = new LRUCache({
max: 100,
ttl: 5 * 60 * 1000,
})

const linesToRevs = require('./lines-to-revs.js')

module.exports = async (repo, opts = {}) => {
if (!opts.noGitRevCache) {
const cached = revsCache.get(repo)
Expand All @@ -17,12 +15,8 @@ module.exports = async (repo, opts = {}) => {
}
}

return pinflight(`ls-remote:${repo}`, () =>
spawn(['ls-remote', repo], opts)
.then(({ stdout }) => linesToRevs(stdout.trim().split('\n')))
.then(revs => {
revsCache.set(repo, revs)
return revs
})
)
const { stdout } = await spawn(['ls-remote', repo], opts)
const revs = linesToRevs(stdout.trim().split('\n'))
revsCache.set(repo, revs)
return revs
}
25 changes: 25 additions & 0 deletions test/revs.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,28 @@ t.test('check the revs', async t => {
Object.keys(r.shas).forEach(sha => r.shas[sha].forEach(ref =>
t.equal(r.refs[ref].sha, sha, `shas list is consistent ${ref}`)))
})

t.test('cache prevents repeated git ls-remote calls', async t => {
let callCount = 0
const originalSpawn = require('../lib/spawn.js')

// Mock `spawn` to track calls
require.cache[require.resolve('../lib/spawn.js')].exports = (...args) => {
callCount++
return originalSpawn(...args)
}

// Force reloading revs.js after modifying spawn
delete require.cache[require.resolve('../lib/revs.js')]
const revsModule = require('../lib/revs.js')

await revsModule(repo) // First call should hit `git ls-remote`
await revsModule(repo) // Second call should use cache

t.equal(callCount, 1, 'git ls-remote should be called only once due to caching')

// Restore original spawn.js
t.teardown(() => {
require.cache[require.resolve('../lib/spawn.js')].exports = originalSpawn
})
})

0 comments on commit 618c70c

Please # to comment.