Cache: add the OpenNextCache entrypoint (Cloudflare) - #44
Conversation
commit: |
| const services = config.services ?? []; | ||
| const keptServices = services.filter((service) => !(service.entrypoint && service.service === config.name)); | ||
|
|
||
| if (!config.configPath || keptServices.length === services.length) { | ||
| return { configPath: config.configPath, cleanup: () => {} }; | ||
| } | ||
|
|
||
| // `unsafe` is dropped as wrangler warns about it being experimental, even when it is empty. | ||
| const { unsafe: _unsafe, ...rest } = config as ResolvedConfig & { unsafe?: unknown }; | ||
|
|
||
| // The file has to sit next to the original one: relative paths are resolved from its directory. | ||
| const configPath = path.join(path.dirname(config.configPath), `.wrangler.opennext.${process.pid}.json`); | ||
|
|
||
| fs.writeFileSync(configPath, JSON.stringify({ ...rest, services: keptServices })); |
There was a problem hiding this comment.
🟡 Local dev server can still fail to start when a Wrangler environment is selected
The self-referencing cache binding is only removed from the top level of the configuration (config.services filter at packages/cloudflare/src/utils/wrangler-config.ts:33) while the environment-specific copy of that list is left untouched and the environment is still selected afterwards, so starting the local dev tooling with an environment can still break.
Impact: Developers who run next dev (or opennextjs-cloudflare preview) with a Wrangler environment can get a startup failure instead of a working dev server.
Why filtering only the merged top-level services is not enough
withoutSelfEntrypointServices receives the already-resolved config from unstable_readConfig({ env: environment, ... }) and filters only config.services. The rewritten JSON is then handed back to getPlatformProxy together with environment (packages/cloudflare/src/api/cloudflare-context.ts:358-371 and packages/cloudflare/src/cli/commands/utils/helpers.ts:12-21).
Two possible outcomes, both wrong:
- If the resolved config object still carries the raw
envmap, wrangler re-appliesenv.<name>.services(bindings are non-inheritable, the environment list wins) and the self-entrypoint binding that was just filtered out comes back, sogetPlatformProxyfails exactly as before. - If the resolved config does not carry the
envmap, the rewritten file has noenv.<name>section whileenvironmentis still passed, and wrangler errors with "No environment found in configuration with name ...".
A fix needs to either filter the services of every environment section as well, or stop passing environment when handing over the already-merged rewritten config.
Prompt for agents
withoutSelfEntrypointServices (packages/cloudflare/src/utils/wrangler-config.ts) strips service bindings that point at a named entrypoint of the worker itself, but it only inspects the top-level `services` array of the config returned by `unstable_readConfig({ env })`. Both call sites (packages/cloudflare/src/api/cloudflare-context.ts getCloudflareContextFromWrangler and packages/cloudflare/src/cli/commands/utils/helpers.ts getEnvFromPlatformProxy) then pass the rewritten config file to getPlatformProxy while ALSO still passing `environment`. When a wrangler environment is used, either the environment's own `services` list (which is not filtered) is re-applied on top of the rewritten config, reintroducing the binding that makes getPlatformProxy fail, or the rewritten config has lost the `env` section and wrangler rejects the unknown environment. Decide on a consistent strategy: either filter self-entrypoint services in every `env.*` section too, or, since the written config is already environment-merged, do not pass `environment` to getPlatformProxy when a rewritten config path is used. Add unit coverage for the environment case.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "services": [ | ||
| { | ||
| "binding": "NEXT_CACHE_SERVICE", | ||
| "service": "r2-incremental-cache", | ||
| "entrypoint": "OpenNextCache" | ||
| } | ||
| ], |
There was a problem hiding this comment.
🟡 Cache service binding is missing for the R2 example when an environment is used
The cache binding is declared only at the top level of the R2 example's configuration (services block at examples-cloudflare/overrides/r2-incremental-cache/wrangler.jsonc:11-17) while each named environment declares its own service list, so the binding disappears whenever the example is deployed or previewed with an environment.
Impact: The R2 example run under its e2e or prod environment has no cache service binding, so once the cache override is enabled there the cache calls would fail at runtime.
Wrangler environments do not inherit binding lists
In wrangler, binding sections (services, kv_namespaces, r2_buckets, ...) are not inheritable: when an environment defines its own services (here env.e2e.services with only WORKER_SELF_REFERENCE), the top-level list is replaced entirely. The newly added top-level entry also points at "r2-incremental-cache", while the worker name under the e2e environment is r2-incremental-cache-e2e, so even if it were inherited it would not self-reference correctly (compare with the WORKER_SELF_REFERENCE entry inside env.e2e, which uses the suffixed name).
The binding should be added inside each environment's services list (with the environment-suffixed worker name) rather than only at the top level.
Was this helpful? React with 👍 or 👎 to provide feedback.
Build the OpenNext cache handler function as part of the worker and expose it through a named entrypoint, so the cache can later run behind a service binding - in this worker or in one of its own. - `service-cache.ts` is a `cache` override that speaks the cache handler HTTP API over the `NEXT_CACHE_SERVICE` binding. - `buildCacheFunction` emits the bundle from `beforeServerBundle`, because the worker imports the entrypoint; core's own cache step is skipped. - `runWithCloudflareContext` initialises the Cloudflare context outside of a request, for entrypoints invoked over RPC. The origin is now populated separately, on the first call coming from the fetch handler. - `withoutSelfEntrypointServices` strips the self referencing binding before handing the config to `getPlatformProxy`, which cannot resolve a named entrypoint of the worker it is configuring. - Drop `compile-cache-assets-manifest.ts`, unreferenced. `defineCloudflareConfig` is untouched: the entrypoint is built and exported but nothing routes to it yet.
467f312 to
c200d7a
Compare
e65c368 to
8b63860
Compare
Builds the cache handler function as part of the worker and exposes it through the
OpenNextCachenamed entrypoint.service-cache.tsis acacheoverride speaking the cache handler HTTP API over theNEXT_CACHE_SERVICEbinding.buildCacheFunctionemits the bundle frombeforeServerBundle, because the worker imports the entrypoint; core's own cache step is skipped.runWithCloudflareContextinitialises the Cloudflare context outside of a request, for entrypoints invoked over RPC. The origin is populated separately, on the first call coming from the fetch handler.withoutSelfEntrypointServicesstrips the self referencing binding before handing the config togetPlatformProxy, which cannot resolve a named entrypoint of the worker it is configuring.compile-cache-assets-manifest.ts, unreferenced.Additive.
defineCloudflareConfigis untouched — the entrypoint is built and exported but nothing routes to it yet. It is landed first so that PR 3 can flip core and Cloudflare together without leaving Cloudflare without a working cache.