Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve monorepo DX #2154

Merged
merged 18 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"type": "module",
"scripts": {
"build": "tsup && node scripts/build-check.mjs",
"dev": "tsup --watch ./src",
"dev": "tsup --watch ./src --watch ../../templates/skeleton",
"typecheck": "tsc --noEmit",
"generate:manifest": "node scripts/generate-manifest.mjs",
"test": "cross-env SHOPIFY_UNIT_TEST=1 vitest run --test-timeout=20000",
Expand Down
23 changes: 13 additions & 10 deletions packages/cli/src/commands/hydrogen/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {outputWarn, collectLog} from '@shopify/cli-kit/node/output';
import {fileSize, removeFile} from '@shopify/cli-kit/node/fs';
import {getPackageManager} from '@shopify/cli-kit/node/node-package-manager';
import {commonFlags, flagsToCamelObject} from '../../lib/flags.js';
import {copyDiffBuild, prepareDiffDirectory} from '../../lib/template-diff.js';
import {prepareDiffDirectory} from '../../lib/template-diff.js';
import {hasViteConfig, getViteConfig} from '../../lib/vite-config.js';
import {checkLockfileStatus} from '../../lib/check-lockfile.js';
import {findMissingRoutes} from '../../lib/missing-routes.js';
Expand Down Expand Up @@ -48,11 +48,12 @@ export default class Build extends Command {
const originalDirectory = flags.path
? resolvePath(flags.path)
: process.cwd();
let directory = originalDirectory;

if (flags.diff) {
directory = await prepareDiffDirectory(originalDirectory, flags.watch);
}
const diff = flags.diff
? await prepareDiffDirectory(originalDirectory, flags.watch)
: undefined;

const directory = diff?.targetDirectory ?? originalDirectory;

const buildParams = {
...flagsToCamelObject(flags),
Expand All @@ -65,18 +66,20 @@ export default class Build extends Command {
: await runClassicCompilerBuild(buildParams);

if (buildParams.watch) {
if (flags.diff || result?.close) {
if (diff || result?.close) {
setupResourceCleanup(async () => {
await result?.close();

if (flags.diff) {
await copyDiffBuild(directory, originalDirectory);
if (diff) {
await diff.copyDiffBuild();
await diff.cleanup();
}
});
}
} else {
if (flags.diff) {
await copyDiffBuild(directory, originalDirectory);
if (diff) {
await diff.copyDiffBuild();
await diff.cleanup();
}

// The Remix compiler hangs due to a bug in ESBuild:
Expand Down
21 changes: 13 additions & 8 deletions packages/cli/src/commands/hydrogen/debug/cpu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,25 @@ export default class DebugCpu extends Command {

async run(): Promise<void> {
const {flags} = await this.parse(DebugCpu);
let directory = flags.path ? resolvePath(flags.path) : process.cwd();
const output = resolvePath(directory, flags.output);
const originalDirectory = flags.path
? resolvePath(flags.path)
: process.cwd();

if (flags.diff) {
directory = await prepareDiffDirectory(directory, true);
}
const diff =
flags.build && flags.diff
? await prepareDiffDirectory(originalDirectory, true)
: undefined;

const {close} = await runDebugCpu({
...flagsToCamelObject(flags),
directory,
output,
directory: diff?.targetDirectory ?? originalDirectory,
output: resolvePath(originalDirectory, flags.output),
});

setupResourceCleanup(close);
setupResourceCleanup(async () => {
await close();
await diff?.cleanup();
});
}
}

Expand Down
8 changes: 4 additions & 4 deletions packages/cli/src/commands/hydrogen/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {getViteConfig} from '../../lib/vite-config.js';
import {prepareDiffDirectory} from '../../lib/template-diff.js';
import {hasRemixConfigFile} from '../../lib/remix-config.js';
import {packageManagers} from '../../lib/package-managers.js';
import {setupResourceCleanup} from '../../lib/resource-cleanup.js';

const DEPLOY_OUTPUT_FILE_HANDLE = 'h2_deploy_log.json';

Expand Down Expand Up @@ -150,10 +151,9 @@ export default class Deploy extends Command {
const deploymentOptions = this.flagsToOxygenDeploymentOptions(flags);

if (flags.diff) {
deploymentOptions.path = await prepareDiffDirectory(
deploymentOptions.path,
false,
);
const diff = await prepareDiffDirectory(deploymentOptions.path, false);
deploymentOptions.path = diff.targetDirectory;
setupResourceCleanup(diff.cleanup);
}

await runDeploy(deploymentOptions);
Expand Down
157 changes: 113 additions & 44 deletions packages/cli/src/commands/hydrogen/dev.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
import {fileURLToPath} from 'node:url';
import type {ViteDevServer} from 'vite';
import Command from '@shopify/cli-kit/node/base-command';
import colors from '@shopify/cli-kit/node/colors';
import {joinPath, resolvePath} from '@shopify/cli-kit/node/path';
import {collectLog} from '@shopify/cli-kit/node/output';
import {
type AlertCustomSection,
renderSuccess,
renderInfo,
} from '@shopify/cli-kit/node/ui';
import {AbortError} from '@shopify/cli-kit/node/error';
import {Flags, Config} from '@oclif/core';
import {
enhanceH2Logs,
isH2Verbose,
Expand All @@ -13,20 +24,10 @@ import {
flagsToCamelObject,
overrideFlag,
} from '../../lib/flags.js';
import Command from '@shopify/cli-kit/node/base-command';
import colors from '@shopify/cli-kit/node/colors';
import {resolvePath} from '@shopify/cli-kit/node/path';
import {collectLog} from '@shopify/cli-kit/node/output';
import {type AlertCustomSection, renderSuccess} from '@shopify/cli-kit/node/ui';
import {AbortError} from '@shopify/cli-kit/node/error';
import {Flags, Config} from '@oclif/core';
import {spawnCodegenProcess} from '../../lib/codegen.js';
import {getAllEnvironmentVariables} from '../../lib/environment-variables.js';
import {displayDevUpgradeNotice} from './upgrade.js';
import {
prepareDiffDirectory,
copyShopifyConfig,
} from '../../lib/template-diff.js';
import {prepareDiffDirectory} from '../../lib/template-diff.js';
import {
getDebugBannerLine,
startTunnelAndPushConfig,
Expand All @@ -35,6 +36,7 @@ import {
getDevConfigInBackground,
getUtilityBannerlines,
TUNNEL_DOMAIN,
getRepoMeta,
} from '../../lib/dev-shared.js';
import {getCliCommand} from '../../lib/shell.js';
import {findPort} from '../../lib/find-port.js';
Expand All @@ -46,6 +48,7 @@ import {importVite} from '../../lib/import-utils.js';
import {createEntryPointErrorHandler} from '../../lib/deps-optimizer.js';
import {getCodeFormatOptions} from '../../lib/format-code.js';
import {setupResourceCleanup} from '../../lib/resource-cleanup.js';
import {removeFile} from '@shopify/cli-kit/node/fs';

export default class Dev extends Command {
static descriptionWithMarkdown = `Runs a Hydrogen storefront in a local runtime that emulates an Oxygen worker for development.
Expand Down Expand Up @@ -114,11 +117,12 @@ export default class Dev extends Command {
const originalDirectory = flags.path
? resolvePath(flags.path)
: process.cwd();
let directory = originalDirectory;

if (flags.diff) {
directory = await prepareDiffDirectory(directory, true);
}
const diff = flags.diff
? await prepareDiffDirectory(originalDirectory, true)
: undefined;

const directory = diff?.targetDirectory ?? originalDirectory;

const devParams = {
...flagsToCamelObject(flags),
Expand All @@ -131,11 +135,14 @@ export default class Dev extends Command {
? await runDev(devParams)
: await runClassicCompilerDev(devParams);

setupResourceCleanup(close);
setupResourceCleanup(async () => {
await close();

if (flags.diff) {
await copyShopifyConfig(directory, originalDirectory);
}
if (diff) {
await diff.copyShopifyConfig();
await diff.cleanup();
}
});
}
}

Expand All @@ -154,7 +161,6 @@ type DevOptions = {
debug?: boolean;
sourcemap?: boolean;
inspectorPort?: number;
isLocalDev?: boolean;
customerAccountPush?: boolean;
cliConfig: Config;
verbose?: boolean;
Expand All @@ -174,7 +180,6 @@ export async function runDev({
debug = false,
disableVersionCheck = false,
inspectorPort,
isLocalDev = false,
customerAccountPush: customerAccountPushFlag = false,
cliConfig,
verbose,
Expand Down Expand Up @@ -211,10 +216,12 @@ export async function runDev({

const vite = await importVite(root);

// Allow Vite to read files from the Hydrogen packages in local development.
const fs = isLocalDev
? {allow: [root, fileURLToPath(new URL('../../../../', import.meta.url))]}
: undefined;
const {hydrogenPackagesPath} = getRepoMeta();

if (hydrogenPackagesPath) {
// Force reoptimizing deps without printing the message
await removeFile(joinPath(root, 'node_modules/.vite'));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also remove the node_modules/.vite that now also appears in the skeleton folder when running npm run dev:app or npm run dev within the skeleton folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the one we are removing here. root should point to the skeleton template when running it there or with npm run dev:app 🤔


const customLogger = vite.createLogger();
if (process.env.SHOPIFY_UNIT_TEST) {
Expand All @@ -232,7 +239,13 @@ export async function runDev({
root,
customLogger,
clearScreen: false,
server: {fs, host: host ? true : undefined},
server: {
host: host ? true : undefined,
// Allow Vite to read files from the Hydrogen packages in local development.
fs: hydrogenPackagesPath
? {allow: [root, hydrogenPackagesPath]}
: undefined,
},
plugins: [
{
name: 'hydrogen:cli',
Expand Down Expand Up @@ -293,30 +306,28 @@ export async function runDev({

const h2PluginOptions = h2Plugin.api?.getPluginOptions?.();

const codegenProcess = useCodegen
? spawnCodegenProcess({
rootDirectory: root,
configFilePath: codegenConfigPath,
appDirectory: h2PluginOptions?.remixConfig?.appDirectory,
})
let codegenProcess: ReturnType<typeof spawnCodegenProcess> | undefined;
const setupCodegen = useCodegen
? () => {
codegenProcess?.kill(0);
codegenProcess = spawnCodegenProcess({
rootDirectory: root,
configFilePath: codegenConfigPath,
appDirectory: h2PluginOptions?.remixConfig?.appDirectory,
});
}
: undefined;

// handle unhandledRejection so that the process won't exit
process.on('unhandledRejection', (err) => {
console.log('Unhandled Rejection: ', err);
});
setupCodegen?.();

if (hydrogenPackagesPath) {
setupMonorepoReload(viteServer, hydrogenPackagesPath, setupCodegen);
}

// Store the port passed by the user in the config.
const publicPort =
appPort ?? viteServer.config.server.port ?? DEFAULT_APP_PORT;

// TODO -- Need to change Remix' <Scripts/> component
// const assetsPort = await findPort(publicPort + 100);
// if (assetsPort) {
// // Note: Set this env before loading Remix config!
// process.env.HYDROGEN_ASSET_BASE_URL = buildAssetsUrl(assetsPort);
// }

const [tunnel, cliCommand] = await Promise.all([
backgroundPromise.then(({customerAccountPush, storefrontId}) =>
customerAccountPush
Expand Down Expand Up @@ -407,3 +418,61 @@ function showSuccessBanner({
customSections,
});
}

function setupMonorepoReload(
viteServer: ViteDevServer,
monorepoPackagesPath: string,
setupCodegen?: () => void,
) {
// Note: app code is already tracked by Vite in monorepos
// so there is no need to watch it and do manual work here.
// We need to track, however, code that is not imported in
// the app and manually reload things here.

viteServer.httpServer?.once('listening', () => {
// Watch the Hydrogen plugin for changes and restart Vite server.
// Virtual routes are already tracked by Vite because they are in-app code.
viteServer.watcher.add(
monorepoPackagesPath + 'hydrogen/dist/vite/plugin.js',
);

// Any change in MiniOxygen will overwrite every file in `dist`.
// We watch the plugin file for example and restart Vite server,
// which also restarts the MiniOxygen worker.
// The only exception is worker-entry because it follows a separate
// build in TSUP.
viteServer.watcher.add(
monorepoPackagesPath + 'mini-oxygen/dist/vite/plugin.js',
);
viteServer.watcher.add(
monorepoPackagesPath + 'mini-oxygen/dist/vite/worker-entry.js',
);

// Watch any file in hydrogen-codegen to restart the codegen process.
viteServer.watcher.add(
monorepoPackagesPath + 'hydrogen-codegen/dist/esm/index.js',
);

viteServer.watcher.on('change', async (file) => {
if (file.includes(monorepoPackagesPath)) {
if (file.includes('/packages/hydrogen-codegen/')) {
if (setupCodegen) {
setupCodegen();
renderInfo({
headline: 'The Hydrogen Codegen source has been modified.',
body: 'The codegen process has been restarted.',
});
}
} else {
// Restart Vite server, which also restarts MiniOxygen
await viteServer.restart(true);
console.log('');
renderInfo({
headline: 'The H2O Vite plugins have been modified.',
body: 'The Vite server has been restarted to reflect the changes.',
});
}
}
});
});
}
Loading
Loading