Skip to content

Conversation

BridgeAR
Copy link

@BridgeAR BridgeAR commented Oct 9, 2025

The current implementation is very slow due to having to load all into memory before writing the file. Pacote itself has a command that does that right away as a stream. This improves the performance significantly.

Fixing the test case seemed more tricky than the fix, so I hope you could suggest what to do about the test.

The current implementation is very slow due to having to load all
into memory before writing the file. Pacote itself has a command
that does that right away. This improves the performance
significantly.
@BridgeAR BridgeAR requested a review from a team as a code owner October 9, 2025 12:23
@wraithgar
Copy link
Member

The reason the test fails is because the code is failing. The file is created but it's empty. npm crashes when lib/commands/pack.js tries to run getContents on the returned object, which is no longer a stream.

pacote.tarball.file eventually returns from #toFile:

{
  integrity: 'sha512-...',
  resolved: '/path',
  from: 'file:/path'
}

via

      writer.on('close', () => res({
        integrity: this.integrity && String(this.integrity),
        resolved: this.resolved,
        from: this.from,
      }))

Instead of the tarballStream.

@BridgeAR
Copy link
Author

I believe the issue is around the return of the pack call. I do not think it should return the tarball in memory, which is a breaking change for this module.
Right now, the PR is definitely incomplete. I think it would just be good to have a closer look and see if we could prevent the in memory read.

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