-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Support Vike in autoconfig #11549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Vike in autoconfig #11549
Conversation
🦋 Changeset detectedLatest commit: 8944c77 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 |
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: |
31e30e7 to
739bdad
Compare
b195a2b to
562f687
Compare
|
Claude finished @dario-piotrowicz's task —— View job Changeset ReviewBoth changeset files need improvement:
|
562f687 to
95afb34
Compare
cd1f4df to
b8f4819
Compare
| // Note: the vike create CLI supports a `--cloudflare` argument for creating cloudflare-ready | ||
| // projects, that works great but here we don't want to use that because we do want to | ||
| // exercise the general autoconfig/`wrangler setup` functionality (which amongst other | ||
| // things lets up ensure that we can support the migration of existing vike projects) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to use the flag in C3, and leave the autoconfig stuff for other sites created without it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes definitely, but I think we need to implement a solution that does allow us to run C3 in both case, with and without a template. The template implementation should be the user facing one, while the autoconfig one should be for e2e tests.
Given the experimental state of this in C3 I would progress with just this implementation now and clean things up later.
I've also captured this in: https://jira.cfdata.org/browse/DEVX-2379
what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems a bit icky to add a template just for e2e tests, but if there's no better way then sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I agree 😕
but right now I cannot see a better path forward (that doesn't require a significant restructuring of how we run these tests)
if it helps any we're doing the exact same thing for Next.js 😅: template vs autoconfig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a plan to handle this differently in future? It would be a lot better if we were testing the actual thing we're using in prod.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a plan to handle this differently in future?
Yes, the plan is that, before going GA, we'd switch back to using templates and have the e2e tests run using those.
Ideally we should also have a non user facing (or more precisely... user facing but hidden) way of running C3 that doesn't use templates and we'd run additional e2e tests for those cases as well.
It would be a lot better if we were testing the actual thing we're using in prod.
So we'll be testing both cases, the actual thing that what we use in prod and the non-template variant that is not used in prod but that would help us ensuring that autoconfig keeps working for newer versions of frameworks
Does this sound ok?
b8f4819 to
dceedb6
Compare
6facb41 to
1a5aa32
Compare
1a5aa32 to
8944c77
Compare
Fixes https://jira.cfdata.org/browse/DEVX-2371