Skip to content

Conversation

@isaacroldan
Copy link
Contributor

@isaacroldan isaacroldan commented Jul 30, 2025

WHY are these changes introduced?

Fixes the inconsistency in how extension assets are referenced in the bundling process.

WHAT is this pull request doing?

This PR changes how extension assets are referenced during the bundling process:

  1. Updates manifest generation to use the more explicit module.outputFolderId instead of module.uid for assets folder.
  2. Adds a new outputFolderId getter to the ExtensionInstance class that returns the extension's uid
  3. Refactors the keepBuiltSourcemapsLocally, buildForBundle, and copyIntoBundle methods to use the extension's outputFolderId directly
  4. Removes the now-redundant getOutputFolderId method and simplifies related code

How to test your changes?

  1. Create an app with multiple extensions
  2. Run shopify app deploy to verify extensions are deployed correctly
  3. Verify that sourcemaps are correctly preserved in the extension folder
  4. Try to deploy a recently migrated app: everything should work

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 30, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
78.25% (+0% 🔼)
13140/16792
🟡 Branches
72.29% (+0.03% 🔼)
6396/8848
🟡 Functions 78.36% 3407/4348
🟡 Lines
78.63% (+0% 🔼)
12437/15818
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / extension-instance.ts
82.11% (-0.19% 🔻)
75.91% (-0.52% 🔻)
92.86%
81.82% (-0.2% 🔻)
🟢
... / theme-command.ts
79.22% (-1.86% 🔻)
51.35% (-0.08% 🔻)
76.47%
80.82% (-2.04% 🔻)

Test suite run success

3103 tests passing in 1322 suites.

Report generated by 🧪jest coverage report action from 8d49cb1

@isaacroldan isaacroldan force-pushed the 07-30-use_uid_for_the_assets_folder_always branch from 19f894d to 8d49cb1 Compare July 30, 2025 08:33
@isaacroldan isaacroldan changed the title Use UID for the assets folder always Use UID for the assets folder and manifest Jul 30, 2025
@isaacroldan isaacroldan marked this pull request as ready for review July 30, 2025 08:35
@isaacroldan isaacroldan requested a review from a team as a code owner July 30, 2025 08:35
@github-actions
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

@isaacroldan isaacroldan added this pull request to the merge queue Jul 31, 2025
Merged via the queue into main with commit a488b9f Jul 31, 2025
2 checks passed
@isaacroldan isaacroldan deleted the 07-30-use_uid_for_the_assets_folder_always branch July 31, 2025 08:40
@alexanderMontague
Copy link
Contributor

/snapit

1 similar comment
@alfonso-noriega
Copy link
Contributor

/snapit

@alfonso-noriega alfonso-noriega restored the 07-30-use_uid_for_the_assets_folder_always branch August 4, 2025 14:13
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.

3 participants