Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Fix CORS (Cross-origin resource sharing) errors #1952

Merged
merged 4 commits into from
Jan 25, 2022
Merged

Fix CORS (Cross-origin resource sharing) errors #1952

merged 4 commits into from
Jan 25, 2022

Conversation

karreiro
Copy link
Contributor

@karreiro karreiro commented Jan 21, 2022 •
edited
Loading

WHY are these changes introduced?

Some assets don't load from the LocalAssets middleware because their path doesn't match or is not a static theme path. So, they load from the CDN, and cross-origin resource sharing errors happen.

WHAT is this pull request doing?

Like the CdnFonts middleware, the CdnAssets requests assets to serve them locally. But, it checks if LocalAssets are not handling the file.

How to test your changes?

  • Run shopify theme init to create a new theme
  • Import product data from products.csv in your store
  • Run shopify theme serve
  • Interact with the Classic Varsity Top and observe the browser console

Before this PR:
before

After this PR:
after

Click here to check a details about the 404 error
This error also happens in the preview store: remote

Post-release steps

None.

Update checklist

  • I've added a CHANGELOG entry for this PR (if the change is public-facing)
  • I've considered possible cross-platform impacts (Mac, Linux, Windows).
  • I've left the version number as is (we'll handle incrementing this when releasing).
  • I've included any post-release steps in the section above.

@karreiro karreiro requested review from macournoyer, a team, pepicrft and gonzaloriestra and removed request for a team January 21, 2022 10:11
@karreiro karreiro linked an issue Jan 21, 2022 that may be closed by this pull request
Section Rendering fails due to lack of CORS Access-Control-Allow-Origin header #1466
Closed
macournoyer
macournoyer approved these changes Jan 21, 2022
View reviewed changes
Copy link
Contributor

@macournoyer macournoyer left a comment

Choose a reason for hiding this comment

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

Minor comments. 🔥 👏

lib/shopify_cli/theme/dev_server/cdn_assets.rb Outdated Show resolved Hide resolved
lib/shopify_cli/theme/dev_server/cdn_assets.rb Outdated Show resolved Hide resolved
@karreiro karreiro merged commit 98ca6f5 into main Jan 25, 2022
@karreiro karreiro deleted the fix-1466 branch January 25, 2022 08:43
@hannachen hannachen mentioned this pull request Jan 25, 2022
Merged
karreiro added a commit that referenced this pull request Jan 28, 2022
@karreiro
Revert "Fix CORS (Cross-origin resource sharing) errors (#1952)"
e6e755f
@karreiro karreiro mentioned this pull request Jan 28, 2022
Merged
4 tasks
karreiro added a commit that referenced this pull request Jan 28, 2022
@karreiro
Revert "Fix CORS (Cross-origin resource sharing) errors (#1952)" (#1985)
47fb451
@karreiro karreiro mentioned this pull request Jan 28, 2022
Merged
This was referenced May 29, 2023
Closed
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@macournoyer macournoyer macournoyer approved these changes

@pepicrft pepicrft Awaiting requested review from pepicrft

@gonzaloriestra gonzaloriestra Awaiting requested review from gonzaloriestra

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Section Rendering fails due to lack of CORS Access-Control-Allow-Origin header
2 participants
@karreiro @macournoyer