fix: retrieveCompiledConfig should respect buildOutputPath - #1332
fix: retrieveCompiledConfig should respect buildOutputPath#1332yuanzhixiang wants to merge 2 commits into
Conversation
retrieveCompiledConfig looked for the compiled config under a hardcoded <cwd>/.open-next/.build/, which does not follow the buildOutputPath config. Every command going through it (deploy, preview, upload, populateCache) exited with "Could not find compiled Open Next config" right after a successful build. The compiled path cannot simply be prefixed with buildOutputPath -- that value lives in the very config being loaded. Recompile from the source config instead when the file is missing, which is what build already does.
🦋 Changeset detectedLatest commit: 28aed56 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // The path above does not follow a custom `buildOutputPath`, which moves the compiled | ||
| // config out of `<cwd>/.open-next`. It cannot be resolved here either -- it lives in | ||
| // the very config we are trying to load. Recompile from the source config instead: | ||
| // `compileOpenNextConfig` emits to a temp dir, so nothing lands in the project. | ||
| return compileConfig(undefined); |
There was a problem hiding this comment.
🟡 Deploying without building first no longer tells the user to run a build
When the built output is missing, the command now quietly rebuilds the configuration from source (compileConfig(undefined) at packages/cloudflare/src/cli/commands/utils/utils.ts:105) instead of stopping with the previous "did you run the build command?" message, so a user who forgot to build gets an unrelated later failure or is even asked to create a new project config file mid-deploy.
Impact: Users running deploy/preview/upload/populate-cache before building see confusing downstream errors, and in an interactive terminal the command can prompt for and write a new config file into the project.
Why the fallback swallows the missing-build case
retrieveCompiledConfig previously distinguished "no build output" from other errors and exited with an actionable message. The new fallback cannot distinguish "buildOutputPath moved the compiled config" from "no build was ever run", so both take the recompile path. compileConfig (packages/cloudflare/src/cli/commands/utils/utils.ts:58-90) additionally may call askConfirmation and createOpenNextConfigFile(nextAppDir, ...) which writes open-next.config.ts into the project — a write side effect that deploy/upload never had before. In CI the error becomes "No open-next.config.ts file was found ... run opennextjs-cloudflare migrate", which is misleading when the real problem is a missing build.
A better shape would be to keep the fallback only after confirming a source config exists, and still surface a build-missing warning/error when the produced worker output is absent.
Prompt for agents
In packages/cloudflare/src/cli/commands/utils/utils.ts, retrieveCompiledConfig now falls back to compileConfig(undefined) whenever <cwd>/.open-next/.build/open-next.config.edge.mjs is absent. This conflates two very different situations: (a) a custom buildOutputPath moved the compiled config, and (b) the user never ran the build. Case (b) previously produced a clear actionable error; now it either fails later with an obscure wrangler error, or (in an interactive shell) prompts the user and writes a new open-next.config.ts into their project via createOpenNextConfigFile. Consider only falling back when a source open-next config actually exists (findOpenNextConfig), never creating a config file from these commands, and still emitting the 'did you run the build command?' error when neither compiled nor source config is available.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "@opennextjs/cloudflare": patch | ||
| --- | ||
|
|
||
| Fix `deploy`/`preview`/`upload`/`populateCache` failing when `buildOutputPath` is set |
There was a problem hiding this comment.
🟡 Changeset description does not follow the repository's required message format
The new changeset body starts with a plain sentence instead of the mandated <type>: <imperative title> first line (.changeset/retrieve-compiled-config-build-output-path.md:5), so the generated changelog entry is inconsistent with every other entry.
Impact: The released changelog entry will not carry the required fix: prefix and imperative title format.
Rule reference
CONTRIBUTING.md ("Changeset message format") and AGENTS.md ("Changesets") both require the body to begin with <TYPE>: <TITLE> where TYPE is one of feature | fix | refactor | docs | chore. The current first line is "Fix deploy/preview/upload/populateCache failing when buildOutputPath is set".
| Fix `deploy`/`preview`/`upload`/`populateCache` failing when `buildOutputPath` is set | |
| fix: make `retrieveCompiledConfig` respect `buildOutputPath` |
Was this helpful? React with 👍 or 👎 to provide feedback.
Devin Review flagged that the fallback conflated "buildOutputPath moved the compiled config" with "the build was never run": the latter used to fail with an actionable message, but now took the recompile path and either failed later with an obscure error or, in an interactive shell, prompted to create an open-next.config.ts in the project -- a write side effect deploy never had. Look up the source config with findOpenNextConfig first so compileConfig can never reach its create-a-config branch, and check the resolved output directory for the built worker afterwards. Running these commands without building still reports the same error, whether the source config is missing or the build was never run. Also reformat the changeset to the <TYPE>: <TITLE> format CONTRIBUTING.md requires, and cover retrieveCompiledConfig with unit tests.
|
Thanks — both review findings were valid, addressed in 28aed56. Missing-build case no longer swallowed. The fallback conflated "
Both failure paths keep the original Changeset format. Reformatted to the Also added unit tests for |
1 participant
What
retrieveCompiledConfigresolves the compiled config from a hardcoded path:https://github.com/opennextjs/opennextjs-cloudflare/blob/main/packages/cloudflare/src/cli/commands/utils/utils.ts#L98
<cwd>/.open-next/.build/does not follow thebuildOutputPathconfig, so withbuildOutputPathset, every command that goes through this function —deploy,preview,upload,populateCache— exits immediately after a successful build:buildis unaffected because it compiles the config from source viacompileConfig, which is why this only surfaces at deploy time.Approach
The path can't simply be prefixed with
buildOutputPath: that value lives in the very config being loaded, so it isn't known at this point. Instead, when the compiled file is missing, this falls back to recompiling from the source config — the samecompileConfigpathbuildtakes.compileOpenNextConfigemits toos.tmpdir(), so nothing is written into the project.Kept as a fallback rather than replacing the fast path, so projects without a custom
buildOutputPathbehave exactly as before.If you'd rather have the command layer resolve
buildOutputPathfirst and keep a single load path, I'm happy to rework it that way — just let me know.Related
@opennextjs/awshas the same class of bug ingetBundlerRuntime, which breaks the build under a custombuildOutputPath. Fixed in opennextjs/opennextjs-aws#1208. Both are needed forbuildOutputPathto work end to end — with only one of them applied you get a green build and a broken deploy, or no build at all.Testing
Verified against a monorepo Next.js 16.3.0 app (Turbopack) with
buildOutputPath: ".cache", using both patches:pnpm lint:check,pnpm ts:check,pnpm test(342 tests) andprettier --checkall pass locally.