Skip to content

refactor: split platform-specific code into platform-vercel and platform-cloudflare packages#8838

Open
ovflowd wants to merge 15 commits intomainfrom
refactor/split-apps-by-deployment-target
Open

refactor: split platform-specific code into platform-vercel and platform-cloudflare packages#8838
ovflowd wants to merge 15 commits intomainfrom
refactor/split-apps-by-deployment-target

Conversation

@ovflowd
Copy link
Copy Markdown
Member

@ovflowd ovflowd commented Apr 23, 2026

Extracts all Vercel- and Cloudflare-specific integrations out of apps/site and into dedicated workspace packages (@node-core/platform-vercel, @node-core/platform-cloudflare), selected at build time via NEXT_PUBLIC_DEPLOY_TARGET (set by vercel.json and the OpenNext buildCommand).

Each adapter exports a next.platform.config.mjs with a { nextConfig, aliases, images, mdx } shape that apps/site merges into its Next.js config, MDX plugins, and Playwright config via dynamic import. No-op apps/site/next.platform.config.mjs and apps/site/playwright.platform.config.mjs keep standalone pnpm dev / pnpm build / pnpm deploy working when no target is set.

Removes the last platform forks from apps/siteVERCEL_ENV, OPEN_NEXT_CLOUDFLARE, 'Cloudflare' in global, and PLAYWRIGHT_RUN_CLOUDFLARE_PREVIEW are all gone.

Copilot AI review requested due to automatic review settings April 23, 2026 02:12
@ovflowd ovflowd requested review from a team as code owners April 23, 2026 02:12
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 23, 2026

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

Project Deployment Actions Updated (UTC)
nodejs-org Ready Ready Preview Apr 23, 2026 11:28pm

Request Review

@cursor

This comment was marked as outdated.

@github-actions
Copy link
Copy Markdown
Contributor

👋 Codeowner Review Request

The following codeowners have been identified for the changed files:

Team reviewers: @nodejs/web-infra @nodejs/nodejs-website

Please review the changes when you have a chance. Thank you! 🙏

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.90%. Comparing base (97e28c5) to head (ecf659d).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8838      +/-   ##
==========================================
+ Coverage   73.87%   73.90%   +0.03%     
==========================================
  Files         105      105              
  Lines        8883     8882       -1     
  Branches      326      324       -2     
==========================================
+ Hits         6562     6564       +2     
+ Misses       2320     2317       -3     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ovflowd
Copy link
Copy Markdown
Member Author

ovflowd commented Apr 23, 2026

FYI deployment will only work once this gets merged as we need to change the root directory within vercel.

Comment thread apps/vercel/next.constants.mjs Outdated
Comment thread apps/vercel/turbo.json Outdated
Comment thread apps/vercel/turbo.json Outdated

This comment was marked as low quality.

Copy link
Copy Markdown
Member

@avivkeller avivkeller left a comment

Choose a reason for hiding this comment

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

Really really against the symlinks, so blocking on that regard

Comment thread .github/workflows/build.yml
Comment thread .github/workflows/build.yml
Comment thread .github/workflows/build.yml
Comment thread .github/workflows/build.yml Outdated
Comment thread apps/cloudflare/app/[locale]/[...path] Outdated
Comment thread apps/site/mdx/plugins.mjs Outdated
Comment thread apps/site/next.config.base.mjs Outdated
Comment thread apps/site/next.image.config.mjs Outdated
Comment thread apps/site/package.json Outdated
Comment thread apps/site/tsconfig.json Outdated
@ovflowd
Copy link
Copy Markdown
Member Author

ovflowd commented Apr 23, 2026

Really really against the symlinks, so blocking on that regard

Why so? And what are your alternatives? Because Im unsure if there are any... Without duplicating every file 😅

@ovflowd
Copy link
Copy Markdown
Member Author

ovflowd commented Apr 23, 2026

Aviv, this PR is a PoC no need to go super nitpicky on it. I haven't done myself a round of reviews yet as I just published this PR so we can start analyzing approaches, I might as well convert to a draft.

@avivkeller avivkeller marked this pull request as draft April 23, 2026 11:30
@avivkeller
Copy link
Copy Markdown
Member

Aviv, this PR is a PoC no need to go super nitpicky on it. I haven't done myself a round of reviews yet as I just published this PR so we can start analyzing approaches, I might as well convert to a draft.

Noted, I've converted it to a draft so it's clearer for reviewers

@avivkeller
Copy link
Copy Markdown
Member

Why so? And what are your alternatives?

I'm not sure what the alternatives are, but Symlinks are a, in my opinion, slippery slope of inconsistent behavior across operating systems and build tools. Is there a way to tell Next.js / OpenNext to look at apps/site/ for the actual site, or something?

@ovflowd
Copy link
Copy Markdown
Member Author

ovflowd commented Apr 23, 2026

Why so? And what are your alternatives?

I'm not sure what the alternatives are, but Symlinks are a, in my opinion, slippery slope of inconsistent behavior across operating systems and build tools. Is there a way to tell Next.js / OpenNext to look at apps/site/ for the actual site, or something?

I'm investigating 👀

Platform detection in apps/site was spread across three different
mechanisms: `VERCEL_ENV`, `OPEN_NEXT_CLOUDFLARE`, and a runtime
`'Cloudflare' in global` check in the MDX plugin. Each one carried its
own caveats (the global check blocked tree-shaking; the two env vars
couldn't be compared to "is this build neutral / Vercel / Cloudflare?"
uniformly), and Vercel analytics were imported eagerly in the root
layout even on Cloudflare builds.

Replace all three with a single `NEXT_PUBLIC_DEPLOY_TARGET` env var set
by each deployment wrapper (`vercel.json` -> `vercel`,
`open-next.config.ts` -> `cloudflare`). The `NEXT_PUBLIC_` prefix lets
Next.js inline the value at build time so platform-specific branches are
dead-code-eliminated from non-matching bundles.

Extract the Vercel Analytics + SpeedInsights injection into a
`platform/body-end` slot. The core layout renders `<BodyEnd />` and the
slot's dynamic import resolves only on Vercel builds, so the Vercel
modules no longer ship to Cloudflare.
@ovflowd ovflowd force-pushed the refactor/split-apps-by-deployment-target branch from 659dda2 to d7b6e7e Compare April 23, 2026 16:09
@ovflowd ovflowd changed the title refactor: split site into site/vercel/cloudflare apps refactor: unify platform detection via NEXT_PUBLIC_DEPLOY_TARGET Apr 23, 2026
…orm-cloudflare packages

Why: Upgrading Next.js or any Vercel dep was gated by OpenNext's
compatibility window, platform-specific runtime branches
(VERCEL_ENV, OPEN_NEXT_CLOUDFLARE, 'Cloudflare' in global) were
scattered across the codebase, and apps/site carried deps that only
one deployment used.

This extracts all platform-specific integrations into dedicated
workspace packages selected at build time via NEXT_PUBLIC_DEPLOY_TARGET:

- @node-core/platform-vercel owns Vercel analytics, speed insights,
  and OTel instrumentation.
- @node-core/platform-cloudflare owns the OpenNext config, Wrangler
  config, worker entrypoint (with Sentry), image loader, and the MDX
  flags needed to skip WASM/Twoslash on Cloudflare workers.

Each adapter exports a next.platform.config.mjs with a
{ nextConfig, aliases, images, mdx } contract that apps/site merges
into its Next.js config, MDX plugins, and Playwright config via
dynamic import. A no-op apps/site/next.platform.config.mjs and
apps/site/playwright.platform.config.mjs keep the standalone
pnpm dev / pnpm build paths working when no target is set.

Runtime platform detection (PLAYWRIGHT_RUN_CLOUDFLARE_PREVIEW,
'Cloudflare' in global, OPEN_NEXT_CLOUDFLARE branches) is replaced
with NEXT_PUBLIC_DEPLOY_TARGET selection so the apps/site source
tree has no platform conditionals left.

Docs updated: docs/technologies.md documents the
NEXT_PUBLIC_DEPLOY_TARGET contract; docs/cloudflare-build-and-deployment.md
points at the new package paths; CODEOWNERS moved the Wrangler /
OpenNext ownership to the new packages.
@ovflowd ovflowd changed the title refactor: unify platform detection via NEXT_PUBLIC_DEPLOY_TARGET refactor: split platform-specific code into platform-vercel and platform-cloudflare packages Apr 23, 2026
@ovflowd ovflowd requested a review from avivkeller April 23, 2026 17:45
@ovflowd ovflowd marked this pull request as ready for review April 23, 2026 17:45
Why: Turbopack's `resolveAlias` treats absolute paths as server-relative
(prepending `./`) and rejects them with "server relative imports are not
implemented yet". The `fileURLToPath(new URL(...))` pattern produced
absolute paths that broke the plain `pnpm build` CI job. Project-relative
strings resolve correctly in both Turbopack and webpack.
Comment thread apps/site/next.config.mjs Outdated
Comment thread .github/CODEOWNERS Outdated
Comment thread apps/site/mdx/plugins.mjs Outdated
Comment thread apps/site/package.json Outdated
ovflowd and others added 2 commits April 23, 2026 17:47
…le site deps

Platform packages are internal deployment wrappers, not publishable
artifacts. Moving them out of packages/ keeps the publish workflow's
globs untouched and matches the repo convention (apps/* = internal,
packages/* = published).

Also removes @node-core/platform-vercel and @node-core/platform-cloudflare
from apps/site's dependencies. They're now declared as optional peer
dependencies so:

- A standalone install (no DEPLOY_TARGET) pulls in zero platform deps.
- Vercel's installCommand scopes install to @node-core/website and
  @node-core/platform-vercel (skipping Cloudflare/OpenNext deps).
- The Cloudflare deploy workflow scopes install to @node-core/website
  and @node-core/platform-cloudflare (skipping @vercel/* deps).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…lare

The platform adapter sub-trees moved out of packages/ in the previous
commit. This refreshes the tree diagram so docs aren't misleading for
new contributors.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment thread apps/site/next.platform.config.d.ts
…ontract

Replace the template-literal `await import()` pattern in next.config.mjs,
mdx/plugins.mjs, and playwright.config.ts with a static
`import from '#platform/...'` resolved by Node's subpath imports. Each
platform's build command sets `NODE_OPTIONS=--conditions=<target>` so
the imports map selects the matching @node-core/platform-* package;
without the flag, the site's local no-op file is used.

Consolidate the duplicated PlatformConfig / PlatformPlaywrightConfig
types into apps/site, the contract owner. Platform .mjs files JSDoc
the site types via relative path; platform-side .d.ts duplicates are
removed.

Also: arrow-function instrumentation hooks; prune the redundant
PLAYWRIGHT_BASE_URL env from the Cloudflare Playwright workflow and
config (kept in the Vercel workflow where it carries the dynamic
preview URL).
Comment thread apps/site/next.constants.mjs
Comment thread apps/site/mdx/plugins.mjs
Turbo's persistent-task output buffering swallows wrangler dev's readiness
signal in CI, making Playwright's webServer probe time out at 180s even
though the preview eventually comes up. Move the OpenNext build to a
dedicated CI step so Playwright's webServer can invoke wrangler dev
directly, keeping stdout attached to the CI log and removing the
dependsOn chain indirection.
wrangler's bin is hoisted to apps/cloudflare/node_modules/.bin under
pnpm, not repo-root node_modules/.bin, so the previous direct path
failed with ENOENT in CI. Delegating to the package script via
`pnpm --filter` resolves wrangler from the right workspace and keeps
stdout attached (no turbo buffering).
Copy link
Copy Markdown
Member

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

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

I appreciate where the PR is coming from, however I don't think I am a huge fan of this, having 3 apps being cloudflare, vercel and site seems quite unclear and unintuitive to me (since there is only a single app, site which can have 2 different variations). There are also magic build time resolutions that I feel are very likely to confuse and alienate people not too familiar with the codebase (and me 😅).

Sorry but personally I feel like, instead of making things clearer the changes here introduce clear/strong boundaries between the different versions of the site at the cost of making the project less clear and navigatable (and approachable by newcommers).

If it were me and we wanted to isolate the vercel and cloudflare logic using different packages in the monorepo I think I'd try to keep site being the only app and instead add cloudflare and vercel as actual packages instead and having site depend on those (with some minor exceptions such as wrangler and open-next config files).

Sorry this is my general take on the approach 😓

(PS: if people are happy with this I'll definitely won't try to spot this direction though 🙂)

Comment thread apps/cloudflare/package.json Outdated
"scripts": {
"cloudflare:build:worker": "cd ../site && opennextjs-cloudflare build --openNextConfigPath ../cloudflare/open-next.config.ts --config ../cloudflare/wrangler.jsonc",
"cloudflare:deploy": "cd ../site && opennextjs-cloudflare deploy --openNextConfigPath ../cloudflare/open-next.config.ts --config ../cloudflare/wrangler.jsonc",
"cloudflare:preview": "cd ../site && wrangler dev --config ../cloudflare/wrangler.jsonc",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel that having build/deploy/preview scripts changing your working directory to be very weird/inconvenient 😕

(also, is it necessary? is open-next not smart enough to work from a different directory? if it's not then we should amend that there)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let me 👀

Webpack bundles the top level of each platform's next.platform.config.mjs
into the server output (because apps/site/mdx/plugins.mjs reads .mdx
from it), which meant the Cloudflare build shipped
`createRequire(...).resolve(...)` and `await getDeploymentId()` into the
worker runtime — failing at request time with `m.resolve is not a
function`. Moving the heavy, build-only pieces (`@opennextjs/cloudflare`
imports, `require.resolve`, VERCEL_URL computation) behind async thunks
keeps the module's top level free of Node-runtime-only code. Pair with
the webpack `conditionNames` wiring so `#platform/*` actually resolves
to the target variant at bundle time.
@ovflowd
Copy link
Copy Markdown
Member Author

ovflowd commented Apr 23, 2026

I'd try to keep site being the only app and instead add cloudflare and vercel as actual packages instead and having site depend on those (with some minor exceptions such as wrangler and open-next config files).

This is the exact opposite others asked. To be clear, apps/cloudflare and apps/vercel aren't "apps" or variations of the website, they are "packages", they are under "apps" because @avivkeller asked, as everything under "packages/" is supposed to be published. I'm fine making a new root folder named "platforms"

To also clarify here, @dario-piotrowicz, apps/site is the only package, all that happens is that when building for Vercel or for Cloudflare we tell Node.js to load the platform-specific config files and things that each one of these environments used.

@dario-piotrowicz
Copy link
Copy Markdown
Member

This is the exact opposite others asked.

Oh ok... different people have different preferences 🙈

To be clear, apps/cloudflare and apps/vercel aren't "apps" or variations of the website, they are "packages", they are under "apps" because @avivkeller asked, as everything under "packages/" is supposed to be published. I'm fine making a new root folder named "platforms"

I see... yeah probably platforms would be clearer in my opinion 🤔 , although they are both tied to the site project (they don't make sense as standalone projects)... mh... I can't remember, in turborepo I think you can have packages inside packages... what about having Vercel and Cloudflare inside site/platforms/? (as standalone/isolated packages but inside the site directory)

To also clarify here, @dario-piotrowicz, apps/site is the only package, all that happens is that when building for Vercel or for Cloudflare we tell Node.js to load the platform-specific config files and things that each one of these environments used.

Yes I got that, but the dir structure really doesn't make it clear in my opinion, since site, cloudflare and vercel are sibling directories

… dir

Replace `cd ../site && ...` in the three cloudflare scripts with
`pnpm --filter=@node-core/website exec ...` so pnpm sets cwd transparently
(per Dario's review feedback). Ignore apps/cloudflare/.wrangler, which is
where wrangler v4 writes its state dir relative to the config file.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ovflowd
Copy link
Copy Markdown
Member Author

ovflowd commented Apr 23, 2026

I see... yeah probably platforms would be clearer in my opinion 🤔 , although they are both tied to the site project (they don't make sense as standalone projects)... mh... I can't remember, in turborepo I think you can have packages inside packages... what about having Vercel and Cloudflare inside site/platforms/? (as standalone/isolated packages but inside the site directory)

Im unsure how putting them as subfolders of apps/site makes things better. One of the goals here is that apps/site is the Next.js App, it is agnostic to any platform. But it is wired to support "platform" overrides, that's where the env flags come in, pretty much we're using Node.js to say "for cloudflare, please load the overrides that come from this apps/cloudflare" package. We aren't really building things within apps/cloudflare, nor apps/cloudflare is an actual app, I guess the name apps/ is the confusing factor. Think of apps/cloudflare or apps/vercel as bindings. Im fine moving them under another folder, but I'd like to avoid keep moving them around as @avivkeller said apps/ you said something else, so if we can reach an agreement of preference, that'd be great.

Yes I got that, but the dir structure really doesn't make it clear in my opinion, since site, cloudflare and vercel are sibling directories

I get that. Anyhow, the goal of this PR is to:

  1. installing apps/site doesn't require any dep of apps/cloudflare or apps/vercel, so reduced deps, better control of blast radius, if smth breaks, it must be Next.js-related things or of our sandboxed app.
  2. removes specific code and checks and paths and settings and logical statements regarding platforms. This makes hosting plain Next.js or static exports easier, also allows the codebase to grow to a number of different platforms.
  3. Easier to group all code retaining a given platform. No more search across all packages;apps/site where we have Vercel or Clopudflare or Platform X-specific code, it is all in one place, easy to find and maintain.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ecf659d. Configure here.


- name: Build open-next site
working-directory: apps/site
working-directory: apps/cloudflare
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing NEXT_PUBLIC_DEPLOY_TARGET env in Cloudflare deploy workflow

Low Severity

The Cloudflare deploy workflow's "Build open-next site" step does not set NEXT_PUBLIC_DEPLOY_TARGET or NODE_OPTIONS=--conditions=cloudflare in the step's env block (unlike the Playwright CI workflow which does). It relies entirely on the open-next.config.ts buildCommand setting these via cross-env. This means the outer opennextjs-cloudflare process itself runs without the cloudflare condition, so any imports it resolves before invoking the buildCommand won't see the condition. The Playwright CI workflow correctly sets both in the step env, suggesting the deploy workflow was overlooked.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ecf659d. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants