Skip to content

[eas-cli] Fix metadata:pull discarding screenshots that share a file name - #4182

Open
giaBaoJS wants to merge 1 commit into
expo:mainfrom
giaBaoJS:fix/metadata-pull-duplicate-screenshot-filenames
Open

[eas-cli] Fix metadata:pull discarding screenshots that share a file name#4182
giaBaoJS wants to merge 1 commit into
expo:mainfrom
giaBaoJS:fix/metadata-pull-duplicate-screenshot-filenames

Conversation

@giaBaoJS

Copy link
Copy Markdown

Why

Fixes #3715.

eas metadata:pull silently discards App Store screenshots that share a file name.

App Store Connect does not enforce unique file names within a screenshot set, so two screenshots in the same set can both report e.g. iPhone 5.5 - 2.png. downloadScreenshotAsync resolved every screenshot to store/apple/screenshot/{locale}/{displayType}/{fileName}, which means the duplicates all mapped to the same local path:

// packages/eas-cli/src/metadata/apple/tasks/screenshots.ts
const fileName = screenshot.attributes.fileName || `${String(index + 1).padStart(2, '0')}.png`;
const outputPath = path.join(screenshotsDir, fileName);

Two things went wrong as a result:

  • each writeFile overwrote the previous screenshot, so only the last one survived on disk;
  • the returned relative path was identical, so the generated store.config.json listed the same file several times instead of listing each screenshot.

Pulling a set of 3 screenshots that share a name left 1 file on disk and 3 identical entries in config. Pushing that config back then re-uploaded the same image in place of the others, so the screenshots were lost in App Store Connect too.

How

Track the file names already used within a single screenshot set and give the duplicates a numeric suffix before the extension (home.png, home-2.png, home-3.png), warning when a name is changed so the mismatch with App Store Connect is discoverable.

The suffix is applied only on a genuine collision. This matters because the push side matches local files to remote screenshots by file name (syncScreenshotSetAsync), so renaming unconditionally would break that correspondence and cause spurious re-uploads and deletions on every push. Names that do not collide are passed through untouched.

Uniqueness is scoped per screenshot set, which is exactly the directory the files land in — the same file name in a different display type or locale writes to a different directory and is left alone.

The suffixed name is also used for the placeholder path written when a screenshot is in a broken AWAITING_UPLOAD state and cannot be downloaded, so those entries do not collapse either.

Test Plan

packages/eas-cli/src/metadata/apple/tasks/__tests__/screenshots.test.ts gains six tests:

downloadAsync

  • keeps original file names when they do not collide — asserts both the config entries and the exact writeFile paths keep the unmodified App Store Connect names. This pins down that the fix cannot degenerate into "always suffix".
  • writes distinct files when screenshots share a file name — the regression test. Three screenshots named iPhone 5.5 - 2.png now produce three distinct config entries and three distinct writeFile paths, and the rename is warned about.
  • does not add suffixes for the same file name in different sets — the same name in APP_IPHONE_67 and APP_IPAD_PRO_3GEN_129 writes to separate directories and is left as-is.
  • deduplicates placeholder paths when broken screenshots share a file name — covers the non-download path.

downloadAsync -> uploadAsync (round trip) — runs a pull and then pushes the config it produced back against the same remote set, to verify local↔remote file-name correspondence survives:

  • pushes back a pulled config without re-uploading or deleting anything — with distinct remote names, push is a complete no-op: nothing uploaded, nothing deleted.
  • only re-uploads the renamed duplicate when file names collide remotely — the entry that kept the original name still matches its remote counterpart and is untouched; only the suffixed duplicate is uploaded, and nothing is deleted.

Verified by reverting the source change and keeping the tests: the three collision tests fail (config and writeFile both collapse to the duplicated path), and the two non-collision tests still pass. Restoring the change turns them green.

Full packages/eas-cli suite: 281/283 suites pass, 2399 passed / 4 skipped, +6 tests versus the tree without this change. The two failing suites (src/observe/__tests__/formatCustomEvents.test.ts, src/observe/__tests__/formatEvents.test.ts) fail identically before and after — they are locale-dependent snapshots unrelated to this change. yarn typecheck, yarn lint and yarn fmt:check are clean.

Note on scope

Push matching is keyed on the file name, so it cannot tell two identically named remote screenshots apart — the second one shadows the first in existingByFilename. That is pre-existing and unchanged here; this PR is limited to making metadata:pull stop destroying the local files. Happy to follow up on the push-side matcher separately if that is wanted.

@github-actions

Copy link
Copy Markdown

Subscribed to pull request

File Patterns Mentions
packages/eas-cli/** @douglowder
packages/eas-cli/src/metadata/** @byCedric

Generated by CodeMention

Warning: The preamble and epilogue options in commentConfiguration are deprecated. Use template instead.

…al name on pull

App Store Connect does not enforce unique file names within a screenshot
set. `eas metadata:pull` resolved every screenshot to
`store/apple/screenshot/{locale}/{displayType}/{fileName}`, so screenshots
sharing a name all mapped to the same path: each download overwrote the
previous one and the config listed the same file several times, silently
losing screenshots.

Track the names already used within a set and add a numeric suffix
(`home.png`, `home-2.png`) to the duplicates, warning when a name is
changed. Names that do not collide are left untouched, so they keep
matching their App Store Connect counterpart when the config is pushed
back.

Fixes expo#3715
@giaBaoJS
giaBaoJS force-pushed the fix/metadata-pull-duplicate-screenshot-filenames branch from 96e9e60 to 0b60ed9 Compare August 18, 2026 15:05
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.

Store screenshots break when duplicate names are used in App Store

1 participant