refactor: run build directly on catalyst core application by matthewvolk · Pull Request #2517 · bigcommerce/catalyst

matthewvolk · 2025-08-11T22:14:46Z

What/Why?

Previously, build was running inside of a temporary directory in an effort to abstract away Cloudflare-related configuration files (wrangler.jsonc, open-next.config.ts) and dependencies (@opennextjs/cloudflare). The only artifact that we copied from this temporary directory back into the original Catalyst project was a single worker.js file.

Unfortunately, we discovered that the opennextjs-cloudflare preview command needs access to some of the intermediary files that were being deleted when the temporary folder was deleted at the end of build. So, we had to change course. We are now installing Cloudflare-related dependencies in the CLI (instead of core) and using an esbuild plugin to resolve imports pointing to these dependencies. To make this work, we had to resort to patching @opennextjs/cloudflare, but have started a conversation with Cloudflare to see if we can more reliable support.

Testing

Updated test suite

Migration

N/A

JIRA: CATALYST-1421

vercel · 2025-08-11T22:14:52Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
catalyst-b2b	Error			Aug 18, 2025 9:41pm
catalyst-canary	Error			Aug 18, 2025 9:41pm

3 Skipped Deployments

Project	Deployment	Updated (UTC)
catalyst	Ignored	Aug 18, 2025 9:41pm
catalyst-au	Ignored	Aug 18, 2025 9:41pm
catalyst-uk	Ignored	Aug 18, 2025 9:41pm

jorgemoya · 2025-08-11T22:38:56Z

+      }),
+    };
+
+    const wranglerConfig = {


Are we moving away from templates then? For preview, I need to create a copy of wranger.jsonc too and want to make sure we have one source of truth.

I'm not opinionated on this, and I agree that my only requirement is that we have a single source of truth. If we want, I can read wrangler.jsonc from fs, and pass that to the build function from OpenNext... keep in mind, we also need to modify certain fields in wrangler.jsonc, so maybe we make a lib function to handle reading/writing/etc. the file. The file can have the config as an object, and then methods to write to wrangler.jsonc using fs, both using the same object as a source of truth?

It's fine either way by me, just feel like we should delete the other templates if we're not using them?

jorgemoya · 2025-08-11T22:42:11Z

+  return createConsola({ level: options.verbose ? LogLevels.verbose : LogLevels.info });
+}
+
+export function createExec(options: BuildCommandOptions, exec: Exec = _execa) {


Not sure if these abstraction are worth it, can you provide your reasoning? Maybe they need to be lib utilities?

I'm cool with lib utils - I'm trying to follow some of the principles in this article, so this abstraction does a couple things:

Helps asserts behavior (“verbose uses inherit”), not which process lib (execa, child_process, etc.) we called.

It also encodes the --verbose policy on exec in a single place. Other commands can use createExec and get --verbose logging options for free. No drift across different call sites.

If you still think it's too early of an abstraction, let's discuss; I'm not set in stone by any means, and open to inlining it if you think it's better

Been thinking about this all day.

How are you asserting the behavior? Just curious, seems to be it would look the same with or without the abstraction. I understand we don't want to test implementation details, only outputs, but how do we test the output of the command?

I agree, but thinking about start as an example, we wouldn't really want to hide it away, thinking this is only required here for the moment?

changeset-bot · 2025-08-12T15:18:12Z

⚠️ No Changeset found

Latest commit: aa41902

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

💥 An error occurred when fetching the changed packages and changesets in this PR

Some errors occurred when validating the changesets config:
The package "@bigcommerce/catalyst-core" depends on the ignored package "@bigcommerce/catalyst", but "@bigcommerce/catalyst-core" is not being ignored. Please add "@bigcommerce/catalyst-core" to the `ignore` option.

jorgemoya · 2025-08-12T20:31:42Z

+      }),
+    };
+
+    const wranglerConfig = {


It's fine either way by me, just feel like we should delete the other templates if we're not using them?

jorgemoya · 2025-08-12T20:42:01Z

+};
+
+export function resolveFramework(options: Options, getConfig: Config, logger: Logger) {
+  const config = getConfig();


Missing rootDir being passed to this function.

Which brings up another talking point, do we care about the rootDir option for commands? I know some of them have it, others don't. For my tests, adding this rootDir option was valuable.

jorgemoya · 2025-08-12T20:43:29Z

+import { getProjectConfig } from '../../lib/project-config';
+import { resolveFramework } from '../../lib/resolve-framework';
+
+import { opennextBuild } from './opennext-build';


openNextBuild or opennextBuild? 🤔

jorgemoya · 2025-08-12T21:09:05Z

+  return createConsola({ level: options.verbose ? LogLevels.verbose : LogLevels.info });
+}
+
+export function createExec(options: BuildCommandOptions, exec: Exec = _execa) {


Been thinking about this all day.

How are you asserting the behavior? Just curious, seems to be it would look the same with or without the abstraction. I understand we don't want to test implementation details, only outputs, but how do we test the output of the command?

I agree, but thinking about start as an example, we wouldn't really want to hide it away, thinking this is only required here for the moment?

jorgemoya · 2025-08-12T21:10:34Z

+
+  try {
+    const openNextConfig = {
+      buildCommand: 'dotenv -e .env.local -- node ./scripts/generate.cjs && next build',


Suggested change

buildCommand: 'dotenv -e .env.local -- node ./scripts/generate.cjs && next build',

buildCommand: 'next build',

jorgemoya · 2025-08-12T21:11:42Z

+        `wrangler@${WRANGLER_VERSION}`,
+        'deploy',
+        '--config',
+        join(process.cwd(), '.bigcommerce', 'wrangler.jsonc'),


Is process.cwd() necessary since we define the cwd below?

matthewvolk · 2025-08-19T20:48:46Z

Fixed by #2527

matthewvolk requested a review from a team August 11, 2025 22:14

matthewvolk commented Aug 11, 2025

View reviewed changes

Comment thread packages/cli/src/commands/build/opennext-build.ts

matthewvolk commented Aug 11, 2025

View reviewed changes

Comment thread packages/cli/package.json

jorgemoya reviewed Aug 11, 2025

View reviewed changes

matthewvolk force-pushed the build-refactor branch from 0a35e37 to 573e99b Compare August 12, 2025 15:18

vercel Bot had a problem deploying to Preview – catalyst-canary August 12, 2025 15:18 Failure

vercel Bot had a problem deploying to Preview – catalyst-b2b August 12, 2025 15:18 Failure

vercel Bot had a problem deploying to Preview – catalyst-canary August 12, 2025 15:25 Failure

vercel Bot had a problem deploying to Preview – catalyst-b2b August 12, 2025 15:25 Failure

vercel Bot had a problem deploying to Preview – catalyst-canary August 12, 2025 15:38 Failure

vercel Bot had a problem deploying to Preview – catalyst-b2b August 12, 2025 15:38 Failure

matthewvolk force-pushed the build-refactor branch from 289a874 to 0014f5f Compare August 12, 2025 15:59

vercel Bot had a problem deploying to Preview – catalyst-canary August 12, 2025 15:59 Failure

vercel Bot had a problem deploying to Preview – catalyst-b2b August 12, 2025 15:59 Failure

matthewvolk force-pushed the build-refactor branch from 0014f5f to 837c9e8 Compare August 12, 2025 16:01

vercel Bot had a problem deploying to Preview – catalyst-canary August 12, 2025 16:01 Failure

vercel Bot had a problem deploying to Preview – catalyst-b2b August 12, 2025 16:01 Failure

refactor: run build directly on catalyst core application

ad7bd6a

matthewvolk force-pushed the build-refactor branch from 837c9e8 to ec9bd6a Compare August 12, 2025 19:31

vercel Bot had a problem deploying to Preview – catalyst-canary August 12, 2025 19:31 Failure

vercel Bot had a problem deploying to Preview – catalyst-b2b August 12, 2025 19:31 Failure

vercel Bot had a problem deploying to Preview – catalyst-canary August 12, 2025 19:55 Failure

vercel Bot had a problem deploying to Preview – catalyst-b2b August 12, 2025 19:56 Failure

matthewvolk force-pushed the build-refactor branch from 677f182 to 3f329db Compare August 12, 2025 20:07

vercel Bot had a problem deploying to Preview – catalyst-canary August 12, 2025 20:08 Failure

vercel Bot had a problem deploying to Preview – catalyst-b2b August 12, 2025 20:08 Failure

matthewvolk added 2 commits August 12, 2025 15:10

refactor: move type generation back to core

72a210b

refactor: move resolve-framework to lib

157d53f

refactor: write wrangler.jsonc to .bigcommerce folder

34ca26e

matthewvolk force-pushed the build-refactor branch from 3f329db to 34ca26e Compare August 12, 2025 20:10

vercel Bot had a problem deploying to Preview – catalyst-canary August 12, 2025 20:10 Failure

vercel Bot had a problem deploying to Preview – catalyst-b2b August 12, 2025 20:11 Failure

jorgemoya mentioned this pull request Aug 12, 2025

feat(cli): run opennext-cloudflare preview for catalyst framework #2518

Merged

jorgemoya reviewed Aug 12, 2025

View reviewed changes

fix: clean up

aa41902

matthewvolk marked this pull request as draft August 18, 2025 21:40

vercel Bot had a problem deploying to Preview – catalyst-b2b August 18, 2025 21:40 Failure

vercel Bot had a problem deploying to Preview – catalyst-canary August 18, 2025 21:41 Failure

matthewvolk closed this Aug 19, 2025

	buildCommand: 'dotenv -e .env.local -- node ./scripts/generate.cjs && next build',
	buildCommand: 'next build',

Conversation

matthewvolk commented Aug 11, 2025 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What/Why?

Testing

Migration

Uh oh!

vercel Bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

changeset-bot Bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthewvolk commented Aug 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

matthewvolk commented Aug 11, 2025 •

edited by atlassian Bot

Loading

vercel Bot commented Aug 11, 2025 •

edited

Loading

changeset-bot Bot commented Aug 12, 2025 •

edited

Loading