cli [Theme] New dev command

New dev command for the theme plugin.

Aug 09 '24 11:08 frandiox

Thanks for your contribution!

Depending on what you are working on, you may want to request a review from a Shopify team:

Themes: @shopify/advanced-edits
UI extensions: @shopify/ui-extensions-cli
- Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
Hydrogen: @shopify/hydrogen
Other: @shopify/app-inner-loop

Aug 09 '24 11:08 github-actions[bot]

Coverage report

St.^{:grey_question:}	Category	Percentage	Covered / Total
🟡	Statements	73.28% (+0.6% 🔼)	8209/11202
🟡	Branches	69.69% (+0.25% 🔼)	3990/5725
🟡	Functions	71.85% (+0.36% 🔼)	2149/2991
🟡	Lines	73.61% (+0.58% 🔼)	7764/10548

Show files with reduced coverage 🔻

St.^{:grey_question:}	File	Statements	Branches	Functions	Lines
🟢	`...` / app.test-data.ts	91.35%	92.63% (-0.15% 🔻)	80.77%	90.75%
🔴	`...` / dev.ts	9.52% (-0.21% 🔻)	0% (-1.56% 🔻)	17.65% (+1.86% 🔼)	9.28% (-0.34% 🔻)
🟡	`...` / previewable-extension.ts	77.78% (-11.11% 🔻)	50% (-50% 🔻)	66.67%	75% (-12.5% 🔻)
🟢	`...` / setup-dev-processes.ts	95.24%	71.43% (-8.57% 🔻)	90.91%	94.44%
🔴	`...` / theme-app-extension.ts	37.93% (-3.45% 🔻)	22.22% (-11.11% 🔻)	25%	37.04% (-3.7% 🔻)
🟡	`...` / uninstall-webhook.ts	80%	63.64% (-18.18% 🔻)	75%	75%
🔴	`...` / web.ts	39.29% (-7.14% 🔻)	30.77% (-15.38% 🔻)	50%	42.31% (-7.69% 🔻)
🟢	`...` / Dev.tsx	93.02% (-0.08% 🔻)	77.78%	90.91%	93.83% (-0.08% 🔻)
🔴	`...` / partners-client.ts	26.32% (-0.55% 🔻)	40%	18.18%	25.98% (-0.58% 🔻)
🟢	`...` / ConcurrentOutput.tsx	98.39% (-1.61% 🔻)	90.91% (-4.55% 🔻)	100%	98.33% (-1.67% 🔻)
🔴	`...` / system.ts	31.58% (-1.75% 🔻)	20.69% (-0.74% 🔻)	62.5%	33.33% (-0.95% 🔻)
🟢	`...` / fqdn.ts	85% (-1.05% 🔻)	85.71% (-1.24% 🔻)	87.5%	84.62% (-1.43% 🔻)

Test suite run success

1854 tests passing in 846 suites.

Report generated by 🧪jest coverage report action from 4f4fe2ed036502cf280dc9962da4d2f4f4c0c2a7

Aug 09 '24 11:08 github-actions[bot]

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset. If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

Aug 19 '24 18:08 github-actions[bot]

Differences in type declarations

We 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:

Some seemingly private modules might be re-exported through public modules.
If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/http.d.ts

@@ -1,13 +1,12 @@
 import FormData from 'form-data';
-import nodeFetch, { RequestInfo, RequestInit } from 'node-fetch';
-export { FetchError, Request } from 'node-fetch';
+import { RequestInfo, RequestInit, Response } from 'node-fetch';
+export { FetchError, Request, Response } from 'node-fetch';
 /**
  * Create a new FormData object.
  *
  * @returns A FormData object.
  */
 export declare function formData(): FormData;
-export type Response = Awaited<ReturnType<typeof nodeFetch>>;
 /**
  * An interface that abstracts way node-fetch. When Node has built-in
  * support for "fetch" in the standard library, we can drop the node-fetch

packages/cli-kit/dist/public/node/themes/types.d.ts

@@ -1,8 +1,27 @@
 /// <reference types="node" resolution-mode="require"/>
+/// <reference types="node" resolution-mode="require"/>
+import type { Stats } from 'fs';
 /**
  * {@link Key} represents the unique identifier of a file in a theme.
  */
 export type Key = string;
+interface ThemeFSEventPayload {
+    fileKey: Key;
+    onContent: (fn: (content: string) => void) => void;
+    onSync: (fn: () => void) => void;
+}
+export type ThemeFSEventName = 'add' | 'change' | 'unlink';
+export type ThemeFSEvent<T extends ThemeFSEventName> = T extends 'unlink' ? {
+    type: 'unlink';
+    payload: Omit<ThemeFSEventPayload, 'onContent'>;
+} : {
+    type: 'add' | 'change';
+    payload: ThemeFSEventPayload;
+};
+interface AddEventListener {
+    (eventName: 'unlink', cb: (params: ThemeFSEvent<'unlink'>['payload']) => void): void;
+    (eventName: 'add' | 'change', cb: (params: ThemeFSEvent<'add'>['payload']) => void): void;
+}
 /**
  * Represents a theme on the file system.
  */
@@ -15,12 +34,16 @@ export interface ThemeFileSystem {
      * Local theme files.
      */
     files: Map<Key, ThemeAsset>;
+    /**
+     * Promise that resolves when all the initial files are found.
+     */
+    ready: () => Promise<void>;
     /**
      * Removes a file from the local disk and updates the themeFileSystem
      *
-     * @param assetKey - The key of the file to remove
+     * @param fileKey - The key of the file to remove
      */
-    delete: (assetKey: string) => Promise<void>;
+    delete: (fileKey: Key) => Promise<void>;
     /**
      * Writes a file to the local disk and updates the themeFileSystem
      *
@@ -32,9 +55,20 @@ export interface ThemeFileSystem {
      * Returns a ThemeAsset representing the file that was read
      * Returns undefined if the file does not exist
      *
-     * @param assetKey - The key of the file to read
+     * @param fileKey - The key of the file to read
+     */
+    read: (fileKey: Key) => Promise<string | Buffer | undefined>;
+    /**
+     * Gets the stats of a file from the local disk and updates the themeFileSystem
+     * Returns undefined if the file does not exist
+     *
+     * @param fileKey - The key of the file to read
      */
-    read: (assetKey: string) => Promise<string | Buffer | undefined>;
+    stat: (fileKey: Key) => Promise<Pick<Stats, 'mtime' | 'size'> | undefined>;
+    /**
+     * Add callbacks to run after certain events are fired.
+     */
+    addEventListener: AddEventListener;
 }
 /**
  * Represents a theme.
@@ -117,4 +151,5 @@ export interface Result {
 export declare enum Operation {
     Delete = "DELETE",
     Upload = "UPLOAD"
-}
\ No newline at end of file
+}
+export {};
\ No newline at end of file

Aug 20 '24 13:08 github-actions[bot]

@karreiro Thanks for the review!

It seems like the hot-reload is not working for sections. I'm getting the following warning when I modify the sections/announcement-bar.liquid file

Fixed in https://github.com/Shopify/cli/pull/4302/commits/bbc0591b21759d8e20364dd17e0336212948cea8 -- I implemented the syncPromise thing as a dummy promise, so I was deleting in-memory stuff too early. I've commented out that code and will wait until the API calls are in place before removing in-memory data.

While debugging that, I also noticed one case of HotReload wasn't working. If you update section names in a JSON file, and later update the section file itself... the server won't know about the changes in the JSON so we need to pass it in replaceTemplates. Fixed in https://github.com/Shopify/cli/pull/4302/commits/9d9e7892b16ca01d6b4f7dde80c5b029b9f30c68

The proxy doesn't seem to load on Safari:

Apparently, Safari adds Upgrade-Insecure-Requests by default, and all requests from <script src="/cdn/..."> tags are transformed to https://127.0.0.1:9292/cdn/.... Fixed in https://github.com/Shopify/cli/pull/4302/commits/99970367680e6ba2432a395afa3c59c1cdabdf04

If you still see the issue it's likely because of some Safari weird cache. Try this:

- Open Safari and go to Preferences > Privacy.
- Click on Manage Website Data.
- Search for 127.0.0.1 and remove any entries related to it.
- Restart Safari

You can also try in localhost instead.

Aug 20 '24 14:08 frandiox

@karreiro I've found another issue related to the predictive search. The /search/suggest endpoint cannot be proxied directly to SFR and needs to be rendered as HTML. However, this request does not contain accept: 'text/html' so it goes by default to the SFR proxy instead of the renderer.

I've added an exception here for now to fix this but perhaps we need a better way?

Aug 21 '24 16:08 frandiox

Differences in type declarations

We 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:

Some seemingly private modules might be re-exported through public modules.
If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/http.d.ts

@@ -1,13 +1,12 @@
 import FormData from 'form-data';
-import nodeFetch, { RequestInfo, RequestInit } from 'node-fetch';
-export { FetchError, Request } from 'node-fetch';
+import { RequestInfo, RequestInit, Response } from 'node-fetch';
+export { FetchError, Request, Response } from 'node-fetch';
 /**
  * Create a new FormData object.
  *
  * @returns A FormData object.
  */
 export declare function formData(): FormData;
-export type Response = Awaited<ReturnType<typeof nodeFetch>>;
 /**
  * An interface that abstracts way node-fetch. When Node has built-in
  * support for "fetch" in the standard library, we can drop the node-fetch

packages/cli-kit/dist/public/node/themes/factories.d.ts

@@ -24,6 +24,7 @@ export interface RemoteBulkUploadResponse {
 }
 export declare function buildTheme(themeJson?: RemoteThemeResponse): Theme | undefined;
 export declare function buildChecksum(asset?: RemoteAssetResponse): Checksum | undefined;
-export declare function buildThemeAsset(asset?: RemoteAssetResponse): ThemeAsset | undefined;
+export declare function buildThemeAsset(asset: undefined): undefined;
+export declare function buildThemeAsset(asset: RemoteAssetResponse): ThemeAsset;
 export declare function buildBulkUploadResults(bulkUploadResponse: RemoteBulkUploadResponse[], assets: AssetParams[]): Result[];
 export {};
\ No newline at end of file

packages/cli-kit/dist/public/node/themes/types.d.ts

@@ -1,8 +1,27 @@
 /// <reference types="node" resolution-mode="require"/>
+/// <reference types="node" resolution-mode="require"/>
+import type { Stats } from 'fs';
 /**
  * {@link Key} represents the unique identifier of a file in a theme.
  */
 export type Key = string;
+export type ThemeFSEventName = 'add' | 'change' | 'unlink';
+interface ThemeFSEventCommonPayload {
+    fileKey: Key;
+    onSync: (fn: () => void) => void;
+}
+type ThemeFSEvent = {
+    type: 'unlink';
+    payload: ThemeFSEventCommonPayload;
+} | {
+    type: 'add' | 'change';
+    payload: ThemeFSEventCommonPayload & {
+        onContent: (fn: (content: string) => void) => void;
+    };
+};
+export type ThemeFSEventPayload<T extends ThemeFSEventName = 'add'> = (ThemeFSEvent & {
+    type: T;
+})['payload'];
 /**
  * Represents a theme on the file system.
  */
@@ -15,12 +34,16 @@ export interface ThemeFileSystem {
      * Local theme files.
      */
     files: Map<Key, ThemeAsset>;
+    /**
+     * Promise that resolves when all the initial files are found.
+     */
+    ready: () => Promise<void>;
     /**
      * Removes a file from the local disk and updates the themeFileSystem
      *
-     * @param assetKey - The key of the file to remove
+     * @param fileKey - The key of the file to remove
      */
-    delete: (assetKey: string) => Promise<void>;
+    delete: (fileKey: Key) => Promise<void>;
     /**
      * Writes a file to the local disk and updates the themeFileSystem
      *
@@ -32,9 +55,22 @@ export interface ThemeFileSystem {
      * Returns a ThemeAsset representing the file that was read
      * Returns undefined if the file does not exist
      *
-     * @param assetKey - The key of the file to read
+     * @param fileKey - The key of the file to read
      */
-    read: (assetKey: string) => Promise<string | Buffer | undefined>;
+    read: (fileKey: Key) => Promise<string | Buffer | undefined>;
+    /**
+     * Gets the stats of a file from the local disk and updates the themeFileSystem
+     * Returns undefined if the file does not exist
+     *
+     * @param fileKey - The key of the file to read
+     */
+    stat: (fileKey: Key) => Promise<Pick<Stats, 'mtime' | 'size'> | undefined>;
+    /**
+     * Add callbacks to run after certain events are fired.
+     */
+    addEventListener: {
+        <T extends ThemeFSEventName>(eventName: T, cb: (params: ThemeFSEventPayload<T>) => void): void;
+    };
 }
 /**
  * Represents a theme.
@@ -117,4 +153,5 @@ export interface Result {
 export declare enum Operation {
     Delete = "DELETE",
     Upload = "UPLOAD"
-}
\ No newline at end of file
+}
+export {};
\ No newline at end of file

Aug 22 '24 12:08 github-actions[bot]