Full autoconfig flow for some projects - #11164
Conversation
🦋 Changeset detectedLatest commit: 238be87 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 |
|
|
||
| export class Astro extends Framework { | ||
| name = "astro"; | ||
| async configure(outputDir: string): Promise<RawConfig> { |
There was a problem hiding this comment.
This does less than C3. In particular, it doesn't set up getPlatformProxy() or the Cloudflare image service. I think we should do those things in the Astro Cloudflare adapter (astro add cloudflare) instead of here.
| $schema: "node_modules/wrangler/config-schema.json", | ||
| name: | ||
| getCIOverrideName() ?? | ||
| autoConfigDetails.packageJson?.name ?? | ||
| dirname(autoConfigDetails.projectPath ?? process.cwd()), | ||
| compatibility_date: getDevCompatibilityDate(undefined), | ||
| observability: { | ||
| enabled: true, | ||
| }, |
There was a problem hiding this comment.
These config properties are common across ~all apps
| /** Whether the framework is used for static generation or fullstack deployment */ | ||
| mode: "static" | "fullstack"; | ||
| }; | ||
| framework?: Framework; |
There was a problem hiding this comment.
@dario-piotrowicz what do you think about this?
| let maybeOutputDir: string | undefined; | ||
| if (args.script) { | ||
| const stats = statSync(args.script); | ||
| if (stats.isDirectory()) { | ||
| maybeOutputDir = args.script; | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not sure about this... This allows wrangler deploy ./public to work, which I think might be an expectation some people have? Do we want to support this? If we do, we likely have to support it in dev and the rest of deploy, too, because the option could be present even if autoconfig doesn't run. How do we resolve conflicts between this and the assets output dir in the config file?
There was a problem hiding this comment.
mh... I am not sure... we never say what you can provide a directory to wrangler deploy and I also find that to be a bit of an ambiguous API (for example someone could thing that you can put the worker code in the directory you provided to wrangler deploy?)...
but I can see where you're coming from...
if we want to have a truly smooth UX we might need to support this 😕
this is probably also something product would chip in as well 🤔
cc. @yomna-shousha @MattieTK
There was a problem hiding this comment.
I am curious how that would conflict with the behavior in this PR: #10016
Are you thinking that autoconfig takes the priority here?
If I am understanding correctly:
I run: wrangler deploy ./public --experimental-autoconfig
Framework detection runs, it finds Astro
It uses ./public as output dir (overriding Astro's default ./dist)
Shows user: "Framework: astro, Build: npm run build, Output: ./public"
Asks: "Deploy with these settings?"
and so on and so forth
Essentially just allowing the user to specify what we would have otherwise detected (potentially correctly or incorrectly for their setup).
There was a problem hiding this comment.
I'm going to remove this logic from this PR—let's discuss and maybe re-apply in a followup
|
|
||
| export interface PackageManager { | ||
| type: "npm" | "yarn" | "pnpm"; | ||
| npx: string; |
Yeah I'm not surprised, I thought of However I am also thinking that it could be useful for testing? (especially if/when we're going to have fixtures for autoconfig). It's totally up to you, for me we can omit it for now and add it later if/when needed 🙂 |
There was a problem hiding this comment.
Sorry I must have misunderstood things in our conversation 😓, if we are reusing c3 to this degree than actually I think it'd make sense to create a private package in the monorepo instead of coping things over. As long as we don't publish that package getting rid of it later if/when needed should be pretty trivial 🤔
But if you already went down this route and want to proceed with it I am also happy with this 🙂
There was a problem hiding this comment.
(if we were to externalize the c3 logic in its own (private) package, we might as well call that package @cloudflare/autoconfig and also contain all the autoconfig logic 🤔 (and we could in the future publish it if that's the direction that product wants))
There was a problem hiding this comment.
This is actually a fairly small subset of C3, and e.g. uses Wrangler's package manager detection rather than C3's, so I think it's okay to keep as-is. We can always split out later if we need to include more.
| /** Whether the framework is used for static generation or fullstack deployment */ | ||
| mode: "static" | "fullstack"; | ||
| }; | ||
| framework?: Framework; |
| let maybeOutputDir: string | undefined; | ||
| if (args.script) { | ||
| const stats = statSync(args.script); | ||
| if (stats.isDirectory()) { | ||
| maybeOutputDir = args.script; | ||
| } | ||
| } |
There was a problem hiding this comment.
mh... I am not sure... we never say what you can provide a directory to wrangler deploy and I also find that to be a bit of an ambiguous API (for example someone could thing that you can put the worker code in the directory you provided to wrangler deploy?)...
but I can see where you're coming from...
if we want to have a truly smooth UX we might need to support this 😕
this is probably also something product would chip in as well 🤔
cc. @yomna-shousha @MattieTK
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
| }); | ||
| await writeFileSync("public/.assetsignore", "_worker.js\n_routes.json"); | ||
| return { | ||
| main: `${outputDir}/dist/_worker.js/index.js`, |
Co-authored-by: Dario Piotrowicz <dario@cloudflare.com>
Co-authored-by: Dario Piotrowicz <dario@cloudflare.com>
|
Failed to automatically backport this PR's changes to Wrangler v3. Please manually create a PR targeting the Depending on your changes, running Notes:
|
dario-piotrowicz
left a comment
There was a problem hiding this comment.
Looks great to me 😄 🚀

Fixes https://jira.cfdata.org/browse/DEVX-2264 & https://jira.cfdata.org/browse/DEVX-2266
This fills out details of
runAutoConfig()andgetDetailsForAutoConfig(). There are several differences from the designed behaviour:options.frameworkbeing just an object containing framework details, it's now a class containing a.configure()function that can be used bygetDetailsForAutoConfig(). This defaults to a "static" implementation that should work for full stack sites, but I think this gives us a clean way to add proper support for full stack frameworks as we migrate them over from C3. I've included support for Astro as an example.index.htmlfile: followup: https://jira.cfdata.org/browse/DEVX-2277 via Detect non-framework static sites #11180projectPathis identical to cwd in all cases. @dario-piotrowicz did you have thoughts here?c3-vendorrather than pulling out to a common location. These are minimal, and in the medium term autoconfig will likely replace the guts of C3 anyway.