Fix additional issues with node builds (#68)

* Fix additional issues with node builds Tested in the standalone nextjs app for sanity's sake, and this is working * comment update
Shopify · Nov 14, 2022 · 2822dee · 2822dee
1 parent 676eb75
commit 2822dee
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 20 deletions.
diff --git a/packages/react/package.json b/packages/react/package.json
@@ -15,43 +15,43 @@
     "dist",
     "storefront.schema.json"
   ],
-  "type": "module",
+  "type": "commonjs",
   "exports": {
     ".": {
       "node": {
         "require": {
           "types": "./dist/types/index.d.cts",
-          "development": "./dist/node-dev/index.cjs",
-          "production": "./dist/node-prod/index.cjs",
-          "default": "./dist/node-prod/index.cjs"
-        },
-        "import": {
-          "types": "./dist/types/index.d.ts",
           "development": "./dist/node-dev/index.js",
           "production": "./dist/node-prod/index.js",
           "default": "./dist/node-prod/index.js"
         },
-        "default": "./dist/node-prod/index.cjs"
+        "import": {
+          "types": "./dist/types/index.d.ts",
+          "development": "./dist/node-dev/index.mjs",
+          "production": "./dist/node-prod/index.mjs",
+          "default": "./dist/node-prod/index.mjs"
+        },
+        "default": "./dist/node-prod/index.js"
       },
       "module": {
         "types": "./dist/types/index.d.ts",
-        "development": "./dist/browser-dev/index.js",
-        "production": "./dist/browser-prod/index.js",
-        "default": "./dist/browser-prod/index.js"
+        "development": "./dist/browser-dev/index.mjs",
+        "production": "./dist/browser-prod/index.mjs",
+        "default": "./dist/browser-prod/index.mjs"
       },
       "import": {
         "types": "./dist/types/index.d.ts",
-        "development": "./dist/browser-dev/index.js",
-        "production": "./dist/browser-prod/index.js",
-        "default": "./dist/browser-prod/index.js"
+        "development": "./dist/browser-dev/index.mjs",
+        "production": "./dist/browser-prod/index.mjs",
+        "default": "./dist/browser-prod/index.mjs"
       },
       "require": {
         "types": "./dist/types/index.d.cts",
-        "development": "./dist/browser-dev/index.cjs",
-        "production": "./dist/browser-prod/index.cjs",
-        "default": "./dist/browser-prod/index.cjs"
+        "development": "./dist/browser-dev/index.js",
+        "production": "./dist/browser-prod/index.js",
+        "default": "./dist/browser-prod/index.js"
       },
-      "default": "./dist/browser-prod/index.js"
+      "default": "./dist/browser-prod/index.mjs"
     },
     "./storefront-api-types": "./dist/types/storefront-api-types.d.ts",
     "./storefront.schema.json": "./storefront.schema.json",

diff --git a/packages/react/vite.config.ts b/packages/react/vite.config.ts
@@ -52,7 +52,13 @@ export default defineConfig(({mode, ssrBuild}) => {
       lib: {
         entry: resolve(__dirname, 'src/index.ts'),
         name: 'hydrogen-react',
-        fileName: (format) => `[name].${format === 'cjs' ? 'c' : ''}js`,
+        /**
+         * we keep the default to commonjs (and package.json#type to "commonjs")
+         * because there are issues when we try to convert to "module"; when we build, there are some files in
+         * `/dist/{folder}/node_modules/{packages and files}` that are esm but end in .js, as that's what Vite is configured to do.
+         * So the package.json doesn't transfer the settings from this package to the nested node_modules package.
+         */
+        fileName: (format) => `[name].${format === 'cjs' ? '' : 'm'}js`,
         formats: ssrBuild ? ['es', 'cjs'] : ['es'],
       },
       sourcemap: true,
@@ -65,7 +71,7 @@ export default defineConfig(({mode, ssrBuild}) => {
             For example, `import {} from '@xstate/react/fsm'` doesn't actually exist in the file path, so we need Vite to process it so it does
             Hypothetically, if they update their package to do so, then we can externalize it at that point.
 
-            Note that this has no effect on the ssr builds; if we need to mark xstate as "not external" there, then we need to use 'ssr.noExternal' https://vitejs.dev/config/ssr-options.html#ssr-noexternal
+            Note that this has no effect on the ssr builds; we need to mark xstate as "not external" in 'ssr.noExternal' https://vitejs.dev/config/ssr-options.html#ssr-noexternal
            */
           if (id.includes('xstate')) {
             return false;
@@ -80,6 +86,17 @@ export default defineConfig(({mode, ssrBuild}) => {
         },
       },
     },
+    ssr: {
+      // for esm builds, we need Vite to process these deps in order to work correctly
+      noExternal: [
+        '@xstate',
+        '@xstate/react',
+        '@xstate/fsm',
+        '@xstate/react/fsm',
+        'use-sync-external-store',
+        'use-isomorphic-layout-effect',
+      ],
+    },
     define: {
       __HYDROGEN_DEV__: mode === 'devbuild' || mode === 'test',
       __HYDROGEN_TEST__: mode === 'test',