-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Support Next.js in autoconfig and also delegate to open-next for wrangler deploy runs executed in open-next projects
#11301
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
Conversation
🦋 Changeset detectedLatest commit: 38859dc 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 |
e1a384a to
31e5296
Compare
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: |
31e5296 to
20d1242
Compare
20d1242 to
a2f24b5
Compare
b1d0f1b to
ba94707
Compare
a2f24b5 to
a087f96
Compare
a087f96 to
db411f0
Compare
|
Converting to draft since the C3 e2es don't currently run in CI and we should enable them before merging and new C3 autoconfig PR |
db411f0 to
9845bde
Compare
|
When I tried this locally, with a new project, I got "✘ [ERROR] The entry-point file at ".open-next/worker.js" was not found.". I needed to do another |
|
The issue is that it doesn't run |
|
Should it change the build command? |
69a48af to
73871b5
Compare
oh... yeah I see the issue.... that's annoying.... I don't think we could change the |
73871b5 to
0927724
Compare
|
I think these changes should be in a separate PR. Will they then override the package.json build command too? |
0d9a598 to
b88b00f
Compare
| * Checks whether a project is an OpenNext one and in such case delegates the deployment to `@opennextjs/cloudflare`. | ||
| * If the project is not an OpenNext one, or the `OPEN_NEXT_DEPLOY` environment variable is set (meaning that `wrangler | ||
| * deploy` has been called from OpenNext itself), then no delegation happens. | ||
| * | ||
| * Why? | ||
| * `wrangler deploy` shouldn't be run on an open-next project, since open-next has its own deploy command, | ||
| * in the interest in having things "just work" we updated `wrangler deploy` to delegate to the open-next | ||
| * deploy command, so that, if a developer runs `npx wrangler deploy` (e.g. like in Workers Builds) on an | ||
| * open-next project that will work just fine. | ||
| * | ||
| * @param projectRoot The path to the project's root | ||
| * @returns true is the deployment has been delegated to open-next, false otherwise |
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.
This is very verbose. Can we simplify to make it clearer what's going on here?
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.
I've simplified this comment: 38859dc
please let me know what you think
| } | ||
| } | ||
|
|
||
| export async function appendToGitIgnore( |
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.
This doesn't do quite the same thing as the existing functions, which check to make sure we're not adding something to the gitignore twice. Is there a reason we no longer need to check that?
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.
If you see the TODO comment I've added at the top of the file I do think that there's some cleanup to do here and code to consolidate 👍
Specifically in regards of the check here, I skipped it to keep things simple, I can add one but I am just using this function for open-next, there I think we'll always end up adding this section anyways because if .open-next as already in the gitignore file then open-next was already being used by the project, but in that case then the app wouldn't require the auto-configuration.
Co-authored-by: Somhairle MacLeòid <[email protected]>
Co-authored-by: Somhairle MacLeòid <[email protected]>
541281c to
3e2729c
Compare
Fixes https://jira.cfdata.org/browse/DEVX-2321