Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix extension devUUID to match remote extension UUID if exists #2895

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

alfonso-noriega
Copy link
Contributor

@alfonso-noriega alfonso-noriega commented Sep 27, 2023

WHY are these changes introduced?

Fixes #461

UI extensions, in order to have live reload, communicate with Web by web sockets. The payload of the WS does not have the uuid updated to the one in the draft. As the UUID is used for reconciliation in EH dev plugin, this results in having two extensions rendered, the one stored in the DB as a draft and the one specified by the WS payload.

WHAT is this pull request doing?

The CLI already resolves the matching extensions, but this matching process was not updating the extension instance. To solve this:

  • Added a new method to the App which updates all extension UUIDS to the ones provided in a map
  • Force the update of the UUIDS after reconciliation of local and remote extensions0

How to test your changes?

  • create an app with a block extension.
  • dev the app and see the extension duplicated at the bottom of the order page.
  • checkout this branch.
  • dev the app again and see just one block extension rendered.

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
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

@github-actions
Copy link
Contributor

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** 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.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 27, 2023

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
73.89% (+0.02% 🔼)
6053/8192
🟡 Branches
71.13% (+0.04% 🔼)
2961/4163
🟡 Functions
72.32% (+0.03% 🔼)
1539/2128
🟡 Lines
75.23% (+0.02% 🔼)
5745/7637

Test suite run success

1433 tests passing in 670 suites.

Report generated by 🧪jest coverage report action from 4ea04a0

Copy link
Contributor

@MitchLillie MitchLillie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎩 'ed, LGTM!

@alfonso-noriega
Copy link
Contributor Author

@Arkham this fix is addressing all the scenarios related to the dev flow, replacing the random dev UUID by the remote UUID right after the matching between local and remote extension happens. There where some changes introduced here which were relying on the .env file values generated after the first deploy leaving un-deployed apps with the UUID not updated causing duplication in preview. Do you think there would be any side effects if the linked changes are removed? could you have a short look at this and give some feedback?

@alfonso-noriega alfonso-noriega added this pull request to the merge queue Sep 29, 2023
Merged via the queue into main with commit 1245a9f Sep 29, 2023
1 check passed
@alfonso-noriega alfonso-noriega deleted the fix_extension_devUUID_upddated_to_remote_UUID branch September 29, 2023 08:14
@shopify-shipit shopify-shipit bot temporarily deployed to nightly September 29, 2023 13:25 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to experimental October 9, 2023 09:43 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to production October 19, 2023 13:18 Inactive
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