feat(cloudflare): add support for user defined entryfile

[alexanderniebuhr]

Changes

• closes Cloudflare adapter is unable to export Durable Object classes #13153
• closes how can i set scheduled function when deploy Cloudflare Workers cron triggers and queues #13838

Testing

• manual testing using new fixture

Docs

• docs(cloudflare): custom entryfile docs#11743

[changeset-bot]

🦋 Changeset detected

Latest commit: ecab6a3

The changes in this PR will be included in the next version bump.

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

[pkg-pr-new]

astro

npm i https://pkg.pr.new/astro@13837

@astrojs/cloudflare

npm i https://pkg.pr.new/@astrojs/cloudflare@13837

@astrojs/netlify

npm i https://pkg.pr.new/@astrojs/netlify@13837

@astrojs/node

npm i https://pkg.pr.new/@astrojs/node@13837

@astrojs/vercel

npm i https://pkg.pr.new/@astrojs/vercel@13837

commit: eb32a48

[florian-lefebvre]

I think I would rename entryfilePath to eg. workerEntrypoint (I think having "entrypoint" is important to stay consistent with other options we have across the public API), and remove entryfileExports to always default to default. Wdyt?

[alexanderniebuhr]

I like the suggestion, however we need to allow the option to add other named exports, since Cloudflare Durable Options needs them.

[florian-lefebvre]

Do you have an example of that? Maybe official cloudflare docs?

[alexanderniebuhr]

@florian-lefebvre you can see an example of a whole entryfile here: https://developers.cloudflare.com/durable-objects/get-started/#summary-and-final-code

[florian-lefebvre]

Ah I missed the MyDurableObject export gotcha. Probably out of scope but could there be a way to allow any export in core?

[alexanderniebuhr]

Probably out of scope but could there be a way to allow any export in core?

I can't answer that, but it would be amazing and best if core could either allow all exports, or maybe infer them automatically and process them. Not sure who is the best to ask about it? 🤔

[florian-lefebvre]

cc @ematipico maybe you have context on that?

[ematipico]

I would like to answer your question, however I miss a lot of context of what we're trying to do. For example the description of the PR doesn't explain what this new feature is about, and I can't understand the connection of the feature with "export everything from core". Could you provide more information, please? Happy to help!

[alexanderniebuhr]

@ematipico This PR adds the optional possibility for a user to provide their own serverEntrypoint instead of the default one we ship with the Cloudflare adapter. This is needed for power users and helps with some outstanding issues linked in the PR description. Anyways back to the question, for an adapter to work we need to set serverEntrypoint to the path of a filewith a createExports function in combination with exports inside of setAdapter (https://docs.astro.build/en/reference/adapter-reference/#exports).

A user defined serverEntrypoint could look like this:

import type { SSRManifest } from 'astro';

import { App } from 'astro/app';
import { handle } from '@astrojs/cloudflare/handler'
import { DurableObject } from 'cloudflare:workers';

class MyDurableObject extends DurableObject<Env> {
  constructor(ctx: DurableObjectState, env: Env) {
    // Required, as we're extending the base class.
    super(ctx, env)
  }
}

export function createExports(manifest: SSRManifest) {
	const app = new App(manifest);
	return {
		default: {
			async fetch(request, env, ctx) {
				return handle(manifest, app, request, env, ctx);
			},
		} satisfies ExportedHandler<Env>,
		MyDurableObject: MyDurableObject,
	}
}

As you can see in this example it has default as well ass MyDurableObject exports. That would mean the user needs to define those in the new option flag manually, but it could be many more. Now the question is, hope that clarifies it a bit, if not please let me know:

Can we get rid of the exports option in setAdapter and just handle all exports, or could astro build infer which exports are present in the file and be intelligent about it.

[ematipico]

Can we get rid of the exports option in setAdapter and just handle all exports, or could astro build infer which exports are present in the file and be intelligent about it.

I believe everything is possible, we just need to find a way around it :) Here's the code that handles the exports:

astro/packages/astro/src/core/build/plugins/plugin-ssr.ts

Lines 186 to 195 in fbcfa68

	adapter.exports
	? `const _exports = serverEntrypointModule.createExports(_manifest, _args);`
	: '',
	...(adapter.exports?.map((name) => {
	if (name === 'default') {
	return `export default _exports.default;`;
	} else {
	return `export const ${name} = _exports['${name}'];`;
	}
	}) ?? []),

As you can see, that array is used to generate the proper SSR code (entry.mjs), in particular it generates the bindings that are exported. If you can find a way to import those bindings at runtime using the adapter/App, that would be definitely a killer enhancement!

We could create an option, like blankExport: true, and if enabled it will ignore exports and the adapter will use whatever new code Astro emits from entry.mjs

[alexanderniebuhr]

Cool. Still that is out of scope for this PR. I'll add an issue to track and investigate it.

[ematipico]

Overall, the feature seems good. I left some comments in the code. The changeset is missing, can you please add it?

[florian-lefebvre]

Don't forget the changeset!

[sarah11918]

Quick thoughts on the changeset for your configuration, @alexanderniebuhr

[ematipico]

There's some inconsistency between the docs of the new options and their implementation. To be more precise, workerEntryPoint.exports now says:

By default, this is set to ['default']

I don't know which one is the correct one, and it should be addressed.

[florian-lefebvre]

Some little knits but looks good to me!

[alexanderniebuhr]

I all address all your nits in a second :)

[sarah11918]

Just checking here because there's some discrepency in the docs PR:

1. Did we decide that the new property is namedExports and that it is ONLY non-default exports? (i.e. the "named" ones?)

2. If so, then doesn't it makes more sense for its default to be an empty array?

   (i.e. by default, there are no named exports?) and under the hood ALL the exports are a merging of default exports (not made visible to the user in a config property at all, since there's no reason to) and any additional named exports?

3. Setting a default value of ['defaults'] on namedExports doesn't solve the problem that if you provide something custom, you've "removed" default exports, right? And it would have to be added back in?

   (i.e. I'm thinking instead that default exports are ALWAYS included, that the default of e.g. exportsButNoOneSeesThis is ['defaults'], but this logic happens invisibly for the user?) Then, if there are any items in namedExports, they get added here?)

Just want to make sure I know what's happening here!

[alexanderniebuhr]

Did we decide that the new property is namedExports and that it is ONLY non-default exports? (i.e. the "named" ones?)

Yes that's correct.

If so, then doesn't it makes more sense for its default to be an empty array?

By default it is undefined. Not sure if an empty array is better.

(i.e. by default, there are no named exports?) and under the hood ALL the exports are a merging of default exports (not made visible to the user in a config property at all, since there's no reason to) and any additional named exports?

That explanation is correct.

So yeah default is included always, and user can either add additional named exports or don't do that.

[sarah11918]

By default it is undefined. Not sure if an empty array is better.

Oh, that's fine! I was noticing in the docs PR where the default value was still listed as default (meaning the default exports) and I was wondering why they were mentioned at all in namedExports.

This all makes sense now. 😄

[sarah11918]

Changeset here looks good! (one small missing word).