[Theme] Specify charset for HTML responses #4512
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your contribution! Depending on what you are working on, you may want to request a review from a Shopify team:
|
|
/snapit |
|
🫰✨ Thanks @frandiox! Your snapshot has been published to npm. Test the snapshot by intalling your package globally: pnpm i -g @shopify/[email protected]
|
Coverage report
Test suite run success1894 tests passing in 860 suites. Report generated by 🧪jest coverage report action from dc30dca |
2 tasks
karreiro
approved these changes
Sep 24, 2024
graygilmore
approved these changes
Sep 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
Fixes #4479 again, after being reopened with a slightly different issue.
I'm unsure why, but it looks like SFR doesn't add the charset encoding when called from development. It seems to be adding it to production sites, however. The legacy CLI was also adding this (not sure if manually or by calling the API in a different way). cc @karreiro do you perhaps have more context on this?
Note: SFR and legacy CLI are not adding the charset encoding to JS / CSS assets so I've not changed that 🤔
WHAT is this pull request doing?
For themes that don't specify
<meta charset="utf-8">in the layout, certain characters are not rendered correctly:This PR adds the proper charset to the contet-type for HTML responses so things are rendered correctly:
How to test your changes?
<meta charset="utf-8">from your layout.××hey××Пи好in any section.Without this PR it won't render correctly.
cc @Shopify/advanced-edits