diff --git a/.changeset/wicked-pants-pull.md b/.changeset/wicked-pants-pull.md new file mode 100644 index 00000000000..824b5376b5f --- /dev/null +++ b/.changeset/wicked-pants-pull.md @@ -0,0 +1,6 @@ +--- +'@shopify/cli-kit': patch +'@shopify/app': patch +--- + +Add React deduplication plugin for ESBuild & extensions diff --git a/packages/app/src/cli/constants.ts b/packages/app/src/cli/constants.ts index b371ebef870..441c262dab9 100644 --- a/packages/app/src/cli/constants.ts +++ b/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: { diff --git a/packages/app/src/cli/services/extensions/bundle.test.ts b/packages/app/src/cli/services/extensions/bundle.test.ts index 892cd577062..5c9788c746d 100644 --- a/packages/app/src/cli/services/extensions/bundle.test.ts +++ b/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 () => { diff --git a/packages/app/src/cli/services/extensions/bundle.ts b/packages/app/src/cli/services/extensions/bundle.ts index 7d9dd86f040..d52fd02efa4 100644 --- a/packages/app/src/cli/services/extensions/bundle.ts +++ b/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> | null, options: B } } -function getESBuildOptions(options: BundleOptions): Parameters[0] { +function getESBuildOptions(options: BundleOptions, processEnv = process.env): Parameters[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 }, legalComments: 'none', minify: options.minify, - plugins: getPlugins(), + plugins: getPlugins(options.stdin.resolveDir, processEnv), target: 'es6', resolveExtensions: ['.tsx', '.ts', '.js', '.json', '.esnext', '.mjs', '.ejs'], } @@ -144,7 +148,7 @@ type ESBuildPlugins = Parameters[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()) { @@ -152,9 +156,39 @@ function getPlugins(): ESBuildPlugins { 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 diff --git a/packages/cli-kit/package.json b/packages/cli-kit/package.json index a86014a0792..b19aeb681ff 100644 --- a/packages/cli-kit/package.json +++ b/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" ] } diff --git a/packages/cli-kit/src/private/node/context/utilities.ts b/packages/cli-kit/src/private/node/context/utilities.ts index 065df0a0b6e..8befdb772c1 100644 --- a/packages/cli-kit/src/private/node/context/utilities.ts +++ b/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 */ diff --git a/packages/cli-kit/src/private/node/demo-recorder.ts b/packages/cli-kit/src/private/node/demo-recorder.ts index da7a8d96269..12515dc67d5 100644 --- a/packages/cli-kit/src/private/node/demo-recorder.ts +++ b/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 diff --git a/packages/cli-kit/src/private/node/testing/ui.ts b/packages/cli-kit/src/private/node/testing/ui.ts index 56551926ca9..8f86ef5a943 100644 --- a/packages/cli-kit/src/private/node/testing/ui.ts +++ b/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' diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index a55a8b01f8c..864c44d9511 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/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' diff --git a/packages/cli-kit/src/public/node/cli.ts b/packages/cli-kit/src/public/node/cli.ts index d7fb69ed36e..d45ebc29b48 100644 --- a/packages/cli-kit/src/public/node/cli.ts +++ b/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' diff --git a/packages/cli-kit/src/public/node/context/local.ts b/packages/cli-kit/src/public/node/context/local.ts index ba081cff51b..0a14bed536c 100644 --- a/packages/cli-kit/src/public/node/context/local.ts +++ b/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' diff --git a/packages/cli-kit/src/public/node/context/spin.ts b/packages/cli-kit/src/public/node/context/spin.ts index 1842ad1e075..e2a09d68022 100644 --- a/packages/cli-kit/src/public/node/context/spin.ts +++ b/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' diff --git a/packages/cli-kit/src/public/node/context/utilities.ts b/packages/cli-kit/src/public/node/context/utilities.ts new file mode 100644 index 00000000000..79c847b67a7 --- /dev/null +++ b/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) +} diff --git a/packages/cli-kit/src/public/node/output.ts b/packages/cli-kit/src/public/node/output.ts index 895dc855aaa..cd174a43b8a 100644 --- a/packages/cli-kit/src/public/node/output.ts +++ b/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, diff --git a/packages/cli-kit/src/public/node/ruby.ts b/packages/cli-kit/src/public/node/ruby.ts index fee12cdeb26..792c91e2e92 100644 --- a/packages/cli-kit/src/public/node/ruby.ts +++ b/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'