Merge pull request Shopify#2047 from Shopify/deduplicate-react

Add plugin to deduplicate React dependency in extensions

.changeset/wicked-pants-pull.md

@@ -0,0 +1,6 @@
+---
+'@shopify/cli-kit': patch
+'@shopify/app': patch
+---
+
+Add React deduplication plugin for ESBuild & extensions

packages/app/src/cli/constants.ts

@@ -1,5 +1,9 @@
 import {ExtensionFlavor} from './models/app/template.js'
 
+export const environmentVariableNames = {
+  skipEsbuildReactDedeuplication: 'SHOPIFY_CLI_SKIP_ESBUILD_REACT_DEDUPLICATION',
+}
+
 export const configurationFileNames = {
   app: 'shopify.app.toml',
   extension: {

packages/app/src/cli/services/extensions/bundle.test.ts

@@ -104,6 +104,68 @@ describe('bundleExtension()', () => {
     `)
     const plugins = options.plugins?.map(({name}) => name)
     expect(plugins).toContain('graphql-loader')
+    expect(plugins).toContain('shopify:deduplicate-react')
+  })
+
+  test('can switch off React deduplication', async () => {
+    // Given
+    const extension = await testUIExtension()
+    const stdout: any = {
+      write: vi.fn(),
+    }
+    const stderr: any = {
+      write: vi.fn(),
+    }
+    const app = testApp({
+      directory: '/project',
+      dotenv: {
+        path: '/project/.env',
+        variables: {
+          FOO: 'BAR',
+        },
+      },
+      allExtensions: [extension],
+    })
+    const esbuildWatch = vi.fn()
+    const esbuildDispose = vi.fn()
+    const esbuildRebuild = vi.fn(esbuildResultFixture)
+
+    vi.mocked(esContext).mockResolvedValue({
+      rebuild: esbuildRebuild,
+      watch: esbuildWatch,
+      dispose: esbuildDispose,
+      cancel: vi.fn(),
+      serve: vi.fn(),
+    })
+
+    // When
+    await bundleExtension(
+      {
+        env: app.dotenv?.variables ?? {},
+        outputPath: extension.outputPath,
+        minify: true,
+        environment: 'production',
+        stdin: {
+          contents: 'console.log("mock stdin content")',
+          resolveDir: 'mock/resolve/dir',
+          loader: 'tsx',
+        },
+        stdout,
+        stderr,
+      },
+      {
+        ...process.env,
+        SHOPIFY_CLI_SKIP_ESBUILD_REACT_DEDUPLICATION: 'true',
+      },
+    )
+
+    // Then
+    const call = vi.mocked(esContext).mock.calls[0]!
+    expect(call).not.toBeUndefined()
+    const options = call[0]
+
+    const plugins = options.plugins?.map(({name}) => name)
+    expect(plugins).not.toContain('shopify:deduplicate-react')
   })
 
   test('stops the ESBuild when the abort signal receives an event', async () => {

packages/app/src/cli/services/extensions/bundle.ts

@@ -1,13 +1,16 @@
 import {ExtensionBuildOptions} from '../build/extension.js'
 import {ExtensionInstance} from '../../models/extensions/extension-instance.js'
 import {themeExtensionFiles} from '../../utilities/extensions/theme.js'
+import {environmentVariableNames} from '../../constants.js'
 import {context as esContext, BuildResult, formatMessagesSync} from 'esbuild'
 import {AbortSignal} from '@shopify/cli-kit/node/abort'
 import {copyFile} from '@shopify/cli-kit/node/fs'
 import {joinPath, relativePath} from '@shopify/cli-kit/node/path'
+import {outputDebug} from '@shopify/cli-kit/node/output'
+import {isTruthy} from '@shopify/cli-kit/node/context/utilities'
 import {Writable} from 'stream'
 import {createRequire} from 'module'
-import type {StdinOptions, build as esBuild} from 'esbuild'
+import type {StdinOptions, build as esBuild, Plugin} from 'esbuild'
 
 const require = createRequire(import.meta.url)
 
@@ -46,9 +49,10 @@ export interface BundleOptions {
 /**
  * Invokes ESBuild with the given options to bundle an extension.
  * @param options - ESBuild options
+ * @param processEnv - Environment variables for the running process (not those from .env)
  */
-export async function bundleExtension(options: BundleOptions) {
-  const esbuildOptions = getESBuildOptions(options)
+export async function bundleExtension(options: BundleOptions, processEnv = process.env) {
+  const esbuildOptions = getESBuildOptions(options, processEnv)
   const context = await esContext(esbuildOptions)
   if (options.watch) {
     await context.watch()
@@ -98,7 +102,7 @@ function onResult(result: Awaited<ReturnType<typeof esBuild>> | null, options: B
   }
 }
 
-function getESBuildOptions(options: BundleOptions): Parameters<typeof esContext>[0] {
+function getESBuildOptions(options: BundleOptions, processEnv = process.env): Parameters<typeof esContext>[0] {
   const env: {[variable: string]: string} = options.env
   const define = Object.keys(env || {}).reduce(
     (acc, key) => ({
@@ -119,7 +123,7 @@ function getESBuildOptions(options: BundleOptions): Parameters<typeof esContext>
     },
     legalComments: 'none',
     minify: options.minify,
-    plugins: getPlugins(),
+    plugins: getPlugins(options.stdin.resolveDir, processEnv),
     target: 'es6',
     resolveExtensions: ['.tsx', '.ts', '.js', '.json', '.esnext', '.mjs', '.ejs'],
   }
@@ -144,17 +148,47 @@ type ESBuildPlugins = Parameters<typeof esContext>[0]['plugins']
  * It returns the plugins that should be used with ESBuild.
  * @returns List of plugins.
  */
-function getPlugins(): ESBuildPlugins {
+function getPlugins(resolveDir: string | undefined, processEnv = process.env): ESBuildPlugins {
   const plugins = []
 
   if (isGraphqlPackageAvailable()) {
     const {default: graphqlLoader} = require('@luckycatfactory/esbuild-graphql-loader')
     plugins.push(graphqlLoader())
   }
 
+  const skipReactDeduplication = isTruthy(processEnv[environmentVariableNames.skipEsbuildReactDedeuplication])
+  if (resolveDir && !skipReactDeduplication) {
+    let resolvedReactPath: string | undefined
+    try {
+      resolvedReactPath = require.resolve('react', {paths: [resolveDir]})
+      // eslint-disable-next-line no-catch-all/no-catch-all
+    } catch {
+      // If weren't able to find React, that's fine. It might not be used.
+      outputDebug(`Unable to load React in ${resolveDir}, skipping React de-duplication`)
+    }
+
+    if (resolvedReactPath) {
+      outputDebug(`Deduplicating React dependency for ${resolveDir}, using ${resolvedReactPath}`)
+      plugins.push(deduplicateReactPlugin(resolvedReactPath))
+    }
+  }
+
   return plugins
 }
 
+function deduplicateReactPlugin(resolvedReactPath: string): Plugin {
+  return {
+    name: 'shopify:deduplicate-react',
+    setup({onResolve}) {
+      onResolve({filter: /^react$/}, (args) => {
+        return {
+          path: resolvedReactPath,
+        }
+      })
+    },
+  }
+}
+
 /**
  * Returns true if the "graphql" and "graphql-tag" packages can be
  * resolved. This information is used to determine whether we should

packages/cli-kit/package.json

@@ -86,7 +86,7 @@
               ],
               "static": [
                 "@oclif/core",
-                "../../private/node/context/utilities.js",
+                "./context/utilities.js",
                 "../../private/node/demo-recorder.js"
               ]
             }

packages/cli-kit/src/private/node/context/utilities.ts

@@ -1,13 +1,3 @@
-/**
- * Returns whether an environment variable value represents a truthy value.
- */
-export function isTruthy(variable: string | undefined): boolean {
-  if (!variable) {
-    return false
-  }
-  return ['1', 'true', 'TRUE', 'yes', 'YES'].includes(variable)
-}
-
 /**
  * Returns whether an environment variable has been set and is non-empty
  */

packages/cli-kit/src/private/node/demo-recorder.ts

@@ -1,5 +1,5 @@
-import {isTruthy} from './context/utilities.js'
 import {ConcurrentOutputProps} from './ui/components/ConcurrentOutput.js'
+import {isTruthy} from '../../public/node/context/utilities.js'
 
 interface Event {
   type: string

packages/cli-kit/src/private/node/testing/ui.ts

@@ -1,4 +1,4 @@
-import {isTruthy} from '../context/utilities.js'
+import {isTruthy} from '../../../public/node/context/utilities.js'
 import {Stdout} from '../ui.js'
 import {ReactElement} from 'react'
 import {render as inkRender} from 'ink'

packages/cli-kit/src/public/node/base-command.ts

@@ -4,10 +4,10 @@ import {isDevelopment} from './context/local.js'
 import {addPublicMetadata} from './metadata.js'
 import {AbortError} from './error.js'
 import {renderInfo} from './ui.js'
+import {outputContent, outputInfo, outputToken} from './output.js'
+import {hashString} from './crypto.js'
+import {isTruthy} from './context/utilities.js'
 import {JsonMap} from '../../private/common/json.js'
-import {outputContent, outputInfo, outputToken} from '../../public/node/output.js'
-import {hashString} from '../../public/node/crypto.js'
-import {isTruthy} from '../../private/node/context/utilities.js'
 import {Command} from '@oclif/core'
 import {FlagOutput, Input, ParserOutput, FlagInput, ArgOutput} from '@oclif/core/lib/interfaces/parser.js'
 

packages/cli-kit/src/public/node/cli.ts

@@ -1,4 +1,4 @@
-import {isTruthy} from '../../private/node/context/utilities.js'
+import {isTruthy} from './context/utilities.js'
 import {printEventsJson} from '../../private/node/demo-recorder.js'
 import {Flags} from '@oclif/core'
 

packages/cli-kit/src/public/node/context/local.ts

@@ -1,5 +1,6 @@
 import {isSpin} from './spin.js'
-import {getCIMetadata, isTruthy, isSet, Metadata} from '../../../private/node/context/utilities.js'
+import {isTruthy} from './utilities.js'
+import {getCIMetadata, isSet, Metadata} from '../../../private/node/context/utilities.js'
 import {environmentVariables, pathConstants} from '../../../private/node/constants.js'
 import {fileExists} from '../fs.js'
 import {exec} from '../system.js'

packages/cli-kit/src/public/node/context/spin.ts

@@ -1,8 +1,8 @@
+import {isTruthy} from './utilities.js'
 import {fileExists, readFileSync} from '../fs.js'
-import {isTruthy} from '../../../private/node/context/utilities.js'
 import {environmentVariables} from '../../../private/node/constants.js'
 import {captureOutput} from '../system.js'
-import {outputContent, outputToken} from '../../../public/node/output.js'
+import {outputContent, outputToken} from '../output.js'
 import {getCachedSpinFqdn, setCachedSpinFqdn} from '../../../private/node/context/spin-cache.js'
 import {AbortError} from '../error.js'
 import {Environment, serviceEnvironment} from '../../../private/node/context/service.js'

packages/cli-kit/src/public/node/context/utilities.ts

@@ -0,0 +1,12 @@
+/**
+ * Returns whether an environment variable value represents a truthy value.
+ *
+ * @param variable - Environment variable value to check.
+ * @returns True when the value is truthy, e.g. '1', 'true', etc.
+ */
+export function isTruthy(variable: string | undefined): boolean {
+  if (!variable) {
+    return false
+  }
+  return ['1', 'true', 'TRUE', 'yes', 'YES'].includes(variable)
+}

packages/cli-kit/src/public/node/output.ts

@@ -3,7 +3,7 @@ import {isUnitTest, isVerbose} from './context/local.js'
 import {PackageManager} from './node-package-manager.js'
 import {AbortSignal} from './abort.js'
 import colors from './colors.js'
-import {isTruthy} from '../../private/node/context/utilities.js'
+import {isTruthy} from './context/utilities.js'
 import {
   ColorContentToken,
   CommandContentToken,

packages/cli-kit/src/public/node/ruby.ts

@@ -7,9 +7,9 @@ import {AbortError, AbortSilentError} from './error.js'
 import {getEnvironmentVariables} from './environment.js'
 import {isSpinEnvironment, spinFqdn} from './context/spin.js'
 import {firstPartyDev, useEmbeddedThemeCLI} from './context/local.js'
+import {outputContent, outputToken} from './output.js'
+import {isTruthy} from './context/utilities.js'
 import {pathConstants} from '../../private/node/constants.js'
-import {outputContent, outputToken} from '../../public/node/output.js'
-import {isTruthy} from '../../private/node/context/utilities.js'
 import {coerceSemverVersion} from '../../private/node/semver.js'
 import {CLI_KIT_VERSION} from '../common/version.js'
 import envPaths from 'env-paths'