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

Make the CLI bundle analysis compatible with older Remix versions that don't output a metafile #1357

Merged
merged 1 commit into from
Sep 21, 2023
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
5 changes: 5 additions & 0 deletions .changeset/popular-items-wink.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/cli-hydrogen': patch
---

Make the CLI bundle analysis compatible with older Remix versions that don't output a metafile
126 changes: 91 additions & 35 deletions packages/cli/src/commands/hydrogen/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
getProjectPaths,
getRemixConfig,
handleRemixImportFail,
RemixConfig,
type ServerMode,
} from '../../lib/remix-config.js';
import {deprecated, commonFlags, flagsToCamelObject} from '../../lib/flags.js';
Expand All @@ -32,6 +33,7 @@ import {codegen} from '../../lib/codegen.js';
import {
buildBundleAnalysis,
getBundleAnalysisSummary,
hasMetafile,
} from '../../lib/bundle/analyzer.js';
import {AbortError} from '@shopify/cli-kit/node/error';

Expand Down Expand Up @@ -148,43 +150,22 @@ export async function runBuild({
if (process.env.NODE_ENV !== 'development') {
console.timeEnd(LOG_WORKER_BUILT);
const sizeMB = (await fileSize(buildPathWorkerFile)) / (1024 * 1024);
const bundleAnalysisPath = await buildBundleAnalysis(buildPath);

outputInfo(
outputContent` ${colors.dim(
relativePath(root, buildPathWorkerFile),
)} ${outputToken.link(
colors.yellow(sizeMB.toFixed(2) + ' MB'),
bundleAnalysisPath,
)}\n`,
);

if (bundleStats && sizeMB < MAX_WORKER_BUNDLE_SIZE) {
outputInfo(
outputContent`${
(await getBundleAnalysisSummary(buildPathWorkerFile)) || '\n'
}\n │\n └─── ${outputToken.link(
'Complete analysis: ' + bundleAnalysisPath,
bundleAnalysisPath,
)}\n\n`,
if (await hasMetafile(buildPath)) {
await writeBundleAnalysis(
buildPath,
root,
buildPathWorkerFile,
sizeMB,
bundleStats,
remixConfig,
);
}

if (sizeMB >= MAX_WORKER_BUNDLE_SIZE) {
throw new AbortError(
'🚨 Worker bundle exceeds 10 MB! Oxygen has a maximum worker bundle size of 10 MB.',
outputContent`See the bundle analysis for a breakdown of what is contributing to the bundle size:\n${outputToken.link(
bundleAnalysisPath,
bundleAnalysisPath,
)}`,
);
} else if (sizeMB >= 5) {
outputWarn(
`🚨 Worker bundle exceeds 5 MB! This can delay your worker response.${
remixConfig.serverMinify
? ''
: ' Minify your bundle by adding `serverMinify: true` to remix.config.js.'
}\n`,
} else {
await writeSimpleBuildStatus(
root,
buildPathWorkerFile,
sizeMB,
remixConfig,
);
}
}
Expand Down Expand Up @@ -213,6 +194,81 @@ export async function runBuild({
}
}

async function writeBundleAnalysis(
buildPath: string,
root: string,
buildPathWorkerFile: string,
sizeMB: number,
bundleStats: boolean,
remixConfig: RemixConfig,
) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the bundle analysis logic is moved in here, and only executed if the metafile exists

const bundleAnalysisPath = await buildBundleAnalysis(buildPath);
outputInfo(
outputContent` ${colors.dim(
relativePath(root, buildPathWorkerFile),
)} ${outputToken.link(
colors.yellow(sizeMB.toFixed(2) + ' MB'),
bundleAnalysisPath,
)}\n`,
);

if (bundleStats && sizeMB < MAX_WORKER_BUNDLE_SIZE) {
outputInfo(
outputContent`${
(await getBundleAnalysisSummary(buildPathWorkerFile)) || '\n'
}\n │\n └─── ${outputToken.link(
'Complete analysis: ' + bundleAnalysisPath,
bundleAnalysisPath,
)}\n\n`,
);
}

if (sizeMB >= MAX_WORKER_BUNDLE_SIZE) {
throw new AbortError(
'🚨 Worker bundle exceeds 10 MB! Oxygen has a maximum worker bundle size of 10 MB.',
outputContent`See the bundle analysis for a breakdown of what is contributing to the bundle size:\n${outputToken.link(
bundleAnalysisPath,
bundleAnalysisPath,
)}`,
);
} else if (sizeMB >= 5) {
outputWarn(
`🚨 Worker bundle exceeds 5 MB! This can delay your worker response.${
remixConfig.serverMinify
? ''
: ' Minify your bundle by adding `serverMinify: true` to remix.config.js.'
}\n`,
);
}
}

async function writeSimpleBuildStatus(
root: string,
buildPathWorkerFile: string,
sizeMB: number,
remixConfig: RemixConfig,
) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it doesn't exist, output this simple status, which is essentially what the CLI did prior to the new bundle analytics.

outputInfo(
outputContent` ${colors.dim(
relativePath(root, buildPathWorkerFile),
)} ${colors.yellow(sizeMB.toFixed(2) + ' MB')}\n`,
);

if (sizeMB >= MAX_WORKER_BUNDLE_SIZE) {
throw new AbortError(
'🚨 Worker bundle exceeds 10 MB! Oxygen has a maximum worker bundle size of 10 MB.',
);
} else if (sizeMB >= 5) {
outputWarn(
`🚨 Worker bundle exceeds 5 MB! This can delay your worker response.${
remixConfig.serverMinify
? ''
: ' Minify your bundle by adding `serverMinify: true` to remix.config.js.'
}\n`,
);
}
}

export async function copyPublicFiles(
publicPath: string,
buildPathClient: string,
Expand Down
13 changes: 11 additions & 2 deletions packages/cli/src/lib/bundle/analyzer.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
import {joinPath, dirname} from '@shopify/cli-kit/node/path';
import {fileURLToPath} from 'node:url';
import {writeFile, readFile} from '@shopify/cli-kit/node/fs';
import {writeFile, readFile, fileExists} from '@shopify/cli-kit/node/fs';
import colors from '@shopify/cli-kit/node/colors';

export async function hasMetafile(buildPath: string): Promise<boolean> {
return (
await Promise.all([
fileExists(joinPath(buildPath, 'worker', 'metafile.server.json')),
fileExists(joinPath(buildPath, 'worker', 'metafile.js.json')),
])
).every(Boolean);
}

export async function buildBundleAnalysis(buildPath: string) {
await Promise.all([
writeBundleAnalyzerFile(
Expand Down Expand Up @@ -66,7 +75,7 @@ export async function getBundleAnalysisSummary(bundlePath: string) {
.split('\n')
.filter((line) => {
const match = line.match(
/(.*)\/node_modules\/(react-dom|@remix-run|@shopify\/hydrogen|react-router|react-router-dom)\/(.*)/g,
/(.*)(node_modules\/|server-assets-manifest:|server-entry-module:)(react-dom|@remix-run|@shopify\/hydrogen|react-router|react-router-dom)\/(.*)/g,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An unrelated fix to the Regular expression hiding in the bundle summary Remix/React and other dependencies that are always present.

);

return !match;
Expand Down
Loading