-
Notifications
You must be signed in to change notification settings - Fork 130
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
Introduce new remote renderer for shopify theme dev
#3891
Conversation
Thanks for your contribution! Depending on what you are working on, you may want to request a review from a Shopify team:
|
shopify theme dev
Coverage report
Test suite run success1660 tests passing in 769 suites. Report generated by 🧪jest coverage report action from 39e0f43 |
98fcd46
to
52627f4
Compare
209b8ad
to
05af4fb
Compare
5f429ab
to
4084d31
Compare
We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset. |
packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts
Show resolved
Hide resolved
packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts
Outdated
Show resolved
Hide resolved
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 a couple of comments re: clarity of some naming and for general understanding
None of which are blocking
Thank you for the 🎩 instructions!! |
4084d31
to
39e0f43
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/http.d.ts@@ -1,6 +1,6 @@
import FormData from 'form-data';
import nodeFetch, { RequestInfo, RequestInit } from 'node-fetch';
-export { FetchError } from 'node-fetch';
+export { FetchError, Request } from 'node-fetch';
/**
* Create a new FormData object.
*
|
WHY are these changes introduced?
Fixes https://github.com/Shopify/develop-advanced-edits/issues/210
This PR introduces the module that remotely renders storefronts and will power part of the
shopify theme dev
logic.WHAT is this pull request doing?
The public API introduced by this PR is:
Internally, the way it works is:
theme-environment/storefront-renderer.ts
- handles the rendering logic. It ensures that the request to render is authenticated, the session is refreshed*, and the required headers are being passedtheme-environment/storefront-session.ts
- handles the session/cookies creation to make sure the renderer is rendering the proper development theme with replace-templates capabilities* This will be introduced in a following PR
How to test your changes?
This is an internal module and will be exposed by the proxy, so we need to manually call it:
packages/theme/src/cli/services/dev.ts
dev
with this:pnpm build
shopify-dev theme dev --path ../dawn --dev-preview
Post-release steps
None.
Measuring impact
How do we know this change was effective? Please choose one:
Checklist
dev
ordeploy
have been reflected in the internal flowchart.