Avoid circular imports for HMR (#2014)

* Avoid circular imports to improve HMR in Vite * Changesets * Normalize loader function to enhance JSDoc * Support function return types and fix SerializeForm in JS * Changesets * Update import path in examples * Revert import path in non-diff examples
Shopify · Apr 19, 2024 · ac4e167 · ac4e167
1 parent 7545fad
commit ac4e167
Show file tree

Hide file tree

Showing 15 changed files with 100 additions and 51 deletions.
diff --git a/.changeset/curly-panthers-jam.md b/.changeset/curly-panthers-jam.md
@@ -0,0 +1,5 @@
+---
+'@shopify/cli-hydrogen': patch
+---
+
+Add `@return` JSDoc tag to functions in JavaScript projects.
diff --git a/.changeset/two-geckos-fry.md b/.changeset/two-geckos-fry.md
@@ -0,0 +1,22 @@
+---
+'skeleton': patch
+---
+
+To improve HMR in Vite, move the `useRootLoaderData` function from `app/root.tsx` to a separate file like `app/lib/root-data.ts`. This change avoids circular imports:
+
+```tsx
+// app/lib/root-data.ts
+import {useMatches} from '@remix-run/react';
+import type {SerializeFrom} from '@shopify/remix-oxygen';
+import type {loader} from '~/root';
+
+/**
+ * Access the result of the root loader from a React component.
+ */
+export const useRootLoaderData = () => {
+  const [root] = useMatches();
+  return root?.data as SerializeFrom<typeof loader>;
+};
+```
+
+Import this hook from `~/lib/root-data` instead of `~/root` in your components.
diff --git a/examples/analytics/app/components/Header.tsx b/examples/analytics/app/components/Header.tsx
@@ -2,7 +2,7 @@ import {Await, NavLink} from '@remix-run/react';
 import {Suspense} from 'react';
 import type {HeaderQuery} from 'storefrontapi.generated';
 import type {LayoutProps} from '~/components/Layout';
-import {useRootLoaderData} from '~/root';
+import {useRootLoaderData} from '~/lib/root-data';
 import {unstable_useAnalytics as useAnalytics} from '@shopify/hydrogen';
 
 type HeaderProps = Pick<LayoutProps, 'header' | 'cart' | 'isLoggedIn'>;

diff --git a/examples/analytics/app/routes/cart.tsx b/examples/analytics/app/routes/cart.tsx
@@ -4,7 +4,7 @@ import type {CartQueryDataReturn} from '@shopify/hydrogen';
 import {UNSTABLE_Analytics as Analytics, CartForm} from '@shopify/hydrogen';
 import {json, type ActionFunctionArgs} from '@shopify/remix-oxygen';
 import {CartMain} from '~/components/Cart';
-import {useRootLoaderData} from '~/root';
+import {useRootLoaderData} from '~/lib/root-data';
 
 export const meta: MetaFunction = () => {
   return [{title: `Hydrogen | Cart`}];

diff --git a/examples/custom-cart-method/app/routes/cart.tsx b/examples/custom-cart-method/app/routes/cart.tsx
@@ -8,7 +8,7 @@ import type {
   CartLineUpdateInput,
 } from '@shopify/hydrogen/storefront-api-types';
 import {CartMain} from '~/components/Cart';
-import {useRootLoaderData} from '~/root';
+import {useRootLoaderData} from '~/lib/root-data';
 
 export const meta: MetaFunction = () => {
   return [{title: `Hydrogen | Cart`}];

diff --git a/examples/legacy-customer-account-flow/app/components/Header.tsx b/examples/legacy-customer-account-flow/app/components/Header.tsx
@@ -2,7 +2,7 @@ import {Await, NavLink} from '@remix-run/react';
 import {Suspense} from 'react';
 import type {HeaderQuery} from 'storefrontapi.generated';
 import type {LayoutProps} from './Layout';
-import {useRootLoaderData} from '~/root';
+import {useRootLoaderData} from '~/lib/root-data';
 
 type HeaderProps = Pick<LayoutProps, 'header' | 'cart' | 'isLoggedIn'>;
 

diff --git a/examples/legacy-customer-account-flow/app/routes/cart.tsx b/examples/legacy-customer-account-flow/app/routes/cart.tsx
@@ -4,7 +4,7 @@ import type {CartQueryDataReturn} from '@shopify/hydrogen';
 import {CartForm} from '@shopify/hydrogen';
 import {json, type ActionFunctionArgs} from '@shopify/remix-oxygen';
 import {CartMain} from '~/components/Cart';
-import {useRootLoaderData} from '~/root';
+import {useRootLoaderData} from '~/lib/root-data';
 
 export const meta: MetaFunction = () => {
   return [{title: `Hydrogen | Cart`}];

diff --git a/packages/cli/src/lib/transpile/morph/functions.ts b/packages/cli/src/lib/transpile/morph/functions.ts
@@ -1,4 +1,5 @@
 import type {
+  FunctionDeclaration,
   FunctionLikeDeclaration,
   JSDocableNode,
   VariableStatement,
@@ -20,31 +21,58 @@ export function generateFunctionDocumentation(
 
   generateParameterDocumentation(functionNode, outputDocNode);
 
-  // Add type annotation to exported functions
-  // using the `export const foo: Foo = () => {}` syntax
-  const jsDocs = variableStatement?.getJsDocs()[0];
-  if (jsDocs?.getTags().length === 0) {
+  if (variableStatement) {
+    // Add type annotation to exported functions
+    // using the `export const foo: Foo = () => {}` syntax
     const declaration = variableStatement?.getDeclarations()[0];
-    const type = declaration?.getType().getText();
-    if (type && type !== 'any' && type !== 'unknown') {
-      const tagName = type.startsWith('() =>') ? 'return' : 'type';
-
-      // Some generated docs try to import variables from the same file.
-      // E.g. `<typeof loader>` becomes `<typeof import("....").loader>`
-      const normalizedType = type
-        .replace(/^\(\)\s+=>\s+/, '')
-        .replace(/typeof import\("[^"]+"\)\./, 'typeof ');
-
-      const text =
-        normalizedType === 'SerializeFrom<typeof loader>'
-          ? `{LoaderReturnData}` // Better alias
-          : `{${normalizedType}}`;
-
-      jsDocs.addTag({tagName, text});
+    const jsDocs = variableStatement?.getJsDocs()[0];
+
+    if (jsDocs?.getTags().length === 0) {
+      const type = declaration?.getType().getText();
+
+      if (isAnnotableType(type)) {
+        jsDocs.addTag({
+          tagName: type.startsWith('() =>') ? 'return' : 'type',
+          text: normalizeType(type),
+        });
+      }
+    }
+  } else {
+    // Add return type annotation to plain functions
+    const declaration = functionNode as FunctionDeclaration;
+    const jsDocs = declaration.getJsDocs()[0];
+
+    if (jsDocs?.getTags().length === 0) {
+      const returnType = declaration.getReturnType().getText();
+      if (isAnnotableType(returnType)) {
+        jsDocs.addTag({tagName: 'return', text: normalizeType(returnType)});
+      }
     }
   }
 }
 
+function isAnnotableType(type?: string): type is string {
+  return !!(
+    type &&
+    type !== 'any' &&
+    type !== 'unknown' &&
+    type !== '{}' &&
+    type !== 'boolean'
+  );
+}
+
+function normalizeType(type: string) {
+  // Some generated docs try to import variables from the same file.
+  // E.g. `<typeof loader>` becomes `<typeof import("....").loader>`
+  const normalizedType = type
+    .replace(/^\(\)\s+=>\s+/, '')
+    .replace(/typeof import\("[^"]+"\)\./, 'typeof ');
+
+  return normalizedType === 'SerializeFrom<any>'
+    ? `{SerializeFrom<loader>}` // Fix inference for loaders outisde of routes
+    : `{${normalizedType}}`;
+}
+
 /**
  * Generate @param documentation from function parameters for functionNode, storing it in docNode
  */

diff --git a/packages/cli/src/lib/transpile/morph/typedefs.ts b/packages/cli/src/lib/transpile/morph/typedefs.ts
@@ -56,15 +56,11 @@ export function generateTypeDefs(sourceFile: SourceFile, code: string) {
 
   const knownGenerics: Record<string, string | undefined> = {
     MetaFunction: 'T',
+    SerializeFrom: 'T',
   };
 
   typedefsFromImports.forEach((typeElements, moduleSpecifier) => {
     for (const typeElement of typeElements) {
-      // We only use this in root.tsx and it's better to
-      // reuse the existing LoaderReturnData, so skip it.
-      if (typeElement === 'SerializeFrom') continue;
-
-      // Note: SerializeFrom also needs generic if we stop skipping it.
       const hasGeneric = !!knownGenerics[typeElement];
 
       typedefs.push(

diff --git a/templates/skeleton/app/components/Footer.tsx b/templates/skeleton/app/components/Footer.tsx
@@ -1,6 +1,6 @@
 import {NavLink} from '@remix-run/react';
 import type {FooterQuery, HeaderQuery} from 'storefrontapi.generated';
-import {useRootLoaderData} from '~/root';
+import {useRootLoaderData} from '~/lib/root-data';
 
 export function Footer({
   menu,

diff --git a/templates/skeleton/app/components/Header.tsx b/templates/skeleton/app/components/Header.tsx
@@ -2,7 +2,7 @@ import {Await, NavLink} from '@remix-run/react';
 import {Suspense} from 'react';
 import type {HeaderQuery} from 'storefrontapi.generated';
 import type {LayoutProps} from './Layout';
-import {useRootLoaderData} from '~/root';
+import {useRootLoaderData} from '~/lib/root-data';
 
 type HeaderProps = Pick<LayoutProps, 'header' | 'cart' | 'isLoggedIn'>;
 

diff --git a/templates/skeleton/app/lib/root-data.ts b/templates/skeleton/app/lib/root-data.ts
@@ -0,0 +1,11 @@
+import {useMatches} from '@remix-run/react';
+import type {SerializeFrom} from '@shopify/remix-oxygen';
+import type {loader} from '~/root';
+
+/**
+ * Access the result of the root loader from a React component.
+ */
+export function useRootLoaderData() {
+  const [root] = useMatches();
+  return root?.data as SerializeFrom<typeof loader>;
+}
diff --git a/templates/skeleton/app/root.tsx b/templates/skeleton/app/root.tsx
@@ -1,15 +1,10 @@
 import {useNonce} from '@shopify/hydrogen';
-import {
-  defer,
-  type SerializeFrom,
-  type LoaderFunctionArgs,
-} from '@shopify/remix-oxygen';
+import {defer, type LoaderFunctionArgs} from '@shopify/remix-oxygen';
 import {
   Links,
   Meta,
   Outlet,
   Scripts,
-  useMatches,
   useRouteError,
   useLoaderData,
   ScrollRestoration,
@@ -58,14 +53,6 @@ export function links() {
   ];
 }
 
-/**
- * Access the result of the root loader from a React component.
- */
-export const useRootLoaderData = () => {
-  const [root] = useMatches();
-  return root?.data as SerializeFrom<typeof loader>;
-};
-
 export async function loader({context}: LoaderFunctionArgs) {
   const {storefront, customerAccount, cart} = context;
   const publicStoreDomain = context.env.PUBLIC_STORE_DOMAIN;
@@ -130,7 +117,7 @@ export default function App() {
 
 export function ErrorBoundary() {
   const error = useRouteError();
-  const rootData = useRootLoaderData();
+  const rootData = useLoaderData<typeof loader>();
   const nonce = useNonce();
   let errorMessage = 'Unknown error';
   let errorStatus = 500;

diff --git a/templates/skeleton/app/routes/blogs.$blogHandle._index.tsx b/templates/skeleton/app/routes/blogs.$blogHandle._index.tsx
@@ -7,11 +7,11 @@ export const meta: MetaFunction<typeof loader> = ({data}) => {
   return [{title: `Hydrogen | ${data?.blog.title ?? ''} blog`}];
 };
 
-export const loader = async ({
+export async function loader({
   request,
   params,
   context: {storefront},
-}: LoaderFunctionArgs) => {
+}: LoaderFunctionArgs) {
   const paginationVariables = getPaginationVariables(request, {
     pageBy: 4,
   });
@@ -32,7 +32,7 @@ export const loader = async ({
   }
 
   return json({blog});
-};
+}
 
 export default function Blog() {
   const {blog} = useLoaderData<typeof loader>();

diff --git a/templates/skeleton/app/routes/cart.tsx b/templates/skeleton/app/routes/cart.tsx
@@ -4,7 +4,7 @@ import type {CartQueryDataReturn} from '@shopify/hydrogen';
 import {CartForm} from '@shopify/hydrogen';
 import {json, type ActionFunctionArgs} from '@shopify/remix-oxygen';
 import {CartMain} from '~/components/Cart';
-import {useRootLoaderData} from '~/root';
+import {useRootLoaderData} from '~/lib/root-data';
 
 export const meta: MetaFunction = () => {
   return [{title: `Hydrogen | Cart`}];