-
Notifications
You must be signed in to change notification settings - Fork 136
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 the theme ID mismatch error, where the live theme ID is returned instead of the development theme ID #4759
Conversation
Coverage report
Test suite run success1919 tests passing in 872 suites. Report generated by 🧪jest coverage report action from ec245ae |
Thank you for the fix @karreiro 🥳 . I 🎩 and it works for me as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left 1 suggestion for test name brevity. non blocking
packages/theme/src/cli/utilities/theme-environment/proxy.test.ts
Outdated
Show resolved
Hide resolved
…instead of the development theme ID
f3a4ea8
to
ec245ae
Compare
/snapit |
🫰✨ Thanks @karreiro! Your snapshot has been published to npm. Test the snapshot by intalling your package globally: pnpm i -g @shopify/[email protected]
|
Thank you for the review, everyone!
I was thinking the same thing, @jamesmengo. Currently, that's expected — that's a different limitation in the proxy, as |
WHY are these changes introduced?
Fixes #4634
When the "Sign in with Shop" is activated, the
/account
endpoint behaves differently (returning the live theme ID in the body) impacting the proxy. However, following the redirect maintains the expected theme ID in the session.It's important to note that stores without that feature were not being affected by this, and are not affected by this change as well.
WHAT is this pull request doing?
This PR updates the proxy logic to proxy
/account
requests and avoid the remote-render of the/account
endpoint.This PR also refactors the logic in the proxy to rely on regular expressions to evaluate patterns to centralize the logic for each case on the regular expressions and prevent conditions from growing like
p.startsWith(..) && p.endsWith(..)
.How to test your changes?
shopify theme dev
in a store with "Sign in with Shop" is activatedBefore
After
Post-release steps
N/A
Measuring impact
How do we know this change was effective? Please choose one:
Checklist