Skip to content

fix(esbuild): sort srcFiles so zip checksum is deterministic#7087

Open
lucasgarsha wants to merge 1 commit into
netlify:mainfrom
lucasgarsha:fix/esbuild-deterministic-zip-order
Open

fix(esbuild): sort srcFiles so zip checksum is deterministic#7087
lucasgarsha wants to merge 1 commit into
netlify:mainfrom
lucasgarsha:fix/esbuild-deterministic-zip-order

Conversation

@lucasgarsha

Copy link
Copy Markdown

Fixes #7086.

The esbuild bundler returned srcFiles: [...supportingSrcFiles, ...bundlePaths.keys()] without sorting, so identical function content was archived in a run-dependent order and the zip sha256 changed every build — defeating Netlify's content-addressed function-upload dedup (every function re-uploads on every deploy). The zisi and nft bundlers already .sort() for this exact reason; this aligns esbuild with them.

Change

Add .sort() to the returned srcFiles in packages/zip-it-and-ship-it/src/runtimes/node/bundlers/esbuild/index.ts. Entry order has no runtime effect, so sorting is safe.

Alternative: sort deduplicatedSrcFiles centrally in packages/zip-it-and-ship-it/src/runtimes/node/utils/zip.ts (which already keeps the add loop synchronous "so that the archive's checksum is deterministic") to cover all bundlers at once. I went with the localized esbuild fix to match the existing zisi/nft precedent and keep the change minimal, but happy to switch if maintainers prefer the central approach.

Test

Added a Vitest regression test in tests/esbuild.test.ts that bundles the same function twice with the esbuild bundler while perturbing the source-file discovery order between runs, and asserts a byte-identical zip. It fails on main and passes with this change.

A plain "bundle twice and compare" test isn't sufficient here: within a single warm process the filesystem traversal order is incidentally stable, so identical checksums are produced even without the fix (the existing 'Produces deterministic checksums' test in main.test.ts passes on buggy main for this reason). The non-determinism only manifests across build environments, so the test reverses the discovered srcFiles order on the second bundle to simulate that variance — which is exactly what the sort neutralizes.

Evidence

  • Before: with the discovery order reversed between two builds of identical source, the zip sha differs (ff807ba… vs 2376cf0…) despite identical contents.
  • After: identical sha regardless of discovery order.
  • Real-world: on a ~239-function Next.js site, an identical redeploy went from ~239 function uploads to 1 once entry order was deterministic.

This was originally found against the standalone netlify/zip-it-and-ship-it repo (now archived → redirects here); the affected code is unchanged after the move.

The esbuild bundler returned `srcFiles: [...supportingSrcFiles,
...bundlePaths.keys()]` without sorting, so identical function content was
archived in a run-dependent order and the zip sha256 changed every build.
This defeats Netlify's content-addressed function-upload dedup, causing every
function to re-upload on every deploy.

The `zisi` and `nft` bundlers already `.sort()` their `srcFiles` for exactly
this reason; this aligns the esbuild bundler with them. Entry order has no
runtime effect, so sorting is safe.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lucasgarsha lucasgarsha requested a review from a team as a code owner June 5, 2026 19:21
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Linter diff in the way? Review this PR in Change Stack to focus on meaningful changes and expand context only when needed.

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 86422f52-59ae-4b77-876d-66b79461d7e1

📥 Commits

Reviewing files that changed from the base of the PR and between dc4e6c7 and cf0c1f8.

📒 Files selected for processing (2)
  • packages/zip-it-and-ship-it/src/runtimes/node/bundlers/esbuild/index.ts
  • packages/zip-it-and-ship-it/tests/esbuild.test.ts

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Ensured Node esbuild bundler produces deterministic output by sorting source files consistently.
  • Tests

    • Added test verifying bundle integrity remains identical regardless of source file discovery order.

Walkthrough

The esbuild bundler now sorts the srcFiles array before returning it, ensuring deterministic zip entry ordering. Previously, unsorted supportingSrcFiles concatenated with bundlePaths keys produced non-deterministic zip checksums across builds, defeating upload deduplication. Tests validate byte-identical ZIP output regardless of source file discovery order via SHA1 comparison.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Adds or modifies js files

Suggested reviewers

  • pieh
  • eduardoboucas
  • 43081j
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: sorting srcFiles in esbuild to make zip checksums deterministic, matching the core objective of the PR.
Description check ✅ Passed The description includes motivation, the specific change, testing approach, and evidence. It follows the template's expected structure and provides comprehensive context.
Linked Issues check ✅ Passed The PR fully addresses issue #7086 by adding .sort() to srcFiles in esbuild bundler and includes a regression test verifying deterministic zip order across source file discovery variance.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the non-deterministic zip entry order issue: sorting srcFiles in esbuild and adding a focused regression test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tishd24

tishd24 commented Jun 5, 2026

Copy link
Copy Markdown

Reviewed this — correct and well-scoped. Sorting the combined srcFiles makes the esbuild bundler's archive entry order deterministic, bringing it in line with the zisi and nft bundlers that already .sort() for exactly this reason.

Things I checked that hold up:

  • Locale-independent. The bare Array.prototype.sort() compares by UTF-16 code unit (not locale), so the ordering is stable across build environments regardless of LANG/LC_*. Good that it's the plain .sort() and not localeCompare — that's what keeps it deterministic cross-env.
  • No input mutation. [...supportingSrcFiles, ...bundlePaths.keys()] is a fresh array, so the in-place sort doesn't touch supportingSrcFiles or the map keys.
  • Test. Mocking getSrcFiles to reverse discovery order and asserting an identical SHA-1 across two builds is a direct, minimal proof of the property (reusing the existing computeSha1 helper). vi.hoisted for the shared flag + the afterEach reset keep it from leaking into the other tests.

Minor / optional:

  • supportingSrcFiles and bundlePaths.keys() can overlap; the sort preserves any duplicates (same behavior as before). If de-duping were ever wanted, [...new Set([...])].sort() would cover both — but that's separate from the checksum-determinism goal here and not needed.
  • Worth a quick sanity check that nothing downstream depends on srcFiles ordering (the zisi/nft precedent strongly suggests not).

For real-world context on why this matters: without it, identical source yields a different zip checksum every build, so the content-addressed function upload never dedups and every function re-uploads each deploy. On a site with ~220 functions (~2.5GB) that meant re-uploading the whole set every time, which intermittently exhausted memory in the upload step. Fixing it here at the source removes the need for downstream re-zip workarounds. 🙏

LGTM.

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.

esbuild bundler produces non-deterministic zip entry order → unstable function checksums defeat upload dedup

2 participants