Skip to content

Do not inject hot-reload code into web-pixels-manager sandbox#1568

Merged
Poitrin merged 3 commits intomainfrom
fix-1119-2
Mar 8, 2023
Merged

Do not inject hot-reload code into web-pixels-manager sandbox#1568
Poitrin merged 3 commits intomainfrom
fix-1119-2

Conversation

@Poitrin
Copy link
Copy Markdown
Contributor

@Poitrin Poitrin commented Mar 7, 2023

WHY are these changes introduced?

Fixes #1119 (again)

When HMR code (SSRClient etc.) is injected into the HTML returned by /, no CORS error happens, and HMR is running. However, URL /web-pixels-manager@0.0.219/sandbox/ was being requested too. This URL returns HTML, in which HMR code is injected and tried to be loaded a second time, “thankfully” leading to the CORS error, because the Origin is null.

The URL to be ignored in #1370 has changed unfortunately and is http://127.0.0.1:9292/wpm@…@…/sandbox/ now, so that the regex needs to be updated.

07-22-hpgai-yjlhi

WHAT is this pull request doing?

Updates regex so that HMR code is only injected for all requests returning HTML and whose URL is not matching …/sandbox.

How to test your changes?

  1. Serve your favorite theme with shopify theme dev
  2. Open your browser’s Network tab and make sure that /hot-reload is only called once.
  3. Open your browser’s console and make sure that no CORS errors appear anymore.

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
  • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

@github-actions

This comment has been minimized.

@Poitrin Poitrin requested a review from a team March 7, 2023 12:34
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2023

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 72.85% 4211/5780
🟡 Branches 70.59% 1918/2717
🟡 Functions 69.81% 1103/1580
🟡 Lines 74.12% 4019/5422

Test suite run success

1078 tests passing in 535 suites.

Report generated by 🧪jest coverage report action from e271953

@mgmanzella
Copy link
Copy Markdown
Contributor

mgmanzella commented Mar 7, 2023

HMR is working but im still getting a CORS error

Screenshot 2023-03-07 at 3 45 55 PM
Screenshot 2023-03-07 at 3 46 28 PM

Copy link
Copy Markdown
Contributor

@mgmanzella mgmanzella left a comment

Choose a reason for hiding this comment

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

nvm! tried again today and it worked 🤷

@Poitrin Poitrin merged commit 9c25351 into main Mar 8, 2023
@Poitrin Poitrin deleted the fix-1119-2 branch March 8, 2023 16:00
Poitrin pushed a commit to Shopify/shopify-cli that referenced this pull request Mar 13, 2023
### WHY are these changes introduced?
Backports changes from CLI 3 to CLI 2, until CLI 2 is sunset.

### WHAT is this pull request doing?
Back-port from
* Shopify/cli#1568
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.

[Bug]: shopify theme dev: CORS errors appear in the browser console

2 participants