Skip to content
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

fix(wrangler): only set environment if defined #49

Merged
merged 6 commits into from
Oct 22, 2024
Merged

fix(wrangler): only set environment if defined #49

merged 6 commits into from
Oct 22, 2024

Conversation

atinux
Copy link
Collaborator

@atinux atinux commented Oct 21, 2024

resolves nuxt-hub/core#331

I noticed that using experiment.inlineRouteRules: true create this issue for wrangler, making the bindings unavailable:
CleanShot 2024-10-21 at 14 38 32@2x

When setting the flag, it seems the runtimeConfig.wrangler.environment === '' and the platform proxy does not like it, I updated so it is given only if set.

I also move from hook to hookOnce to avoid keeping a watcher.

@atinux atinux requested a review from pi0 October 21, 2024 12:44
Copy link
Collaborator

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 🙂

but...

I am not very familiar with the nuxt/nuxthub codebases, but (and sorry for being potentially annoying 😅) I feel that if this issue is caused by having runtimeConfig.wrangler.environment === '' then the real problem if somewhere up the stack, as runtimeConfig.wrangler.environment should not be set to an empty string, do you by any chance know where this happens and why?

@atinux
Copy link
Collaborator Author

atinux commented Oct 21, 2024

Actually NuxtHub does not configure the environment at all.

It is related to the inlineRouteRules experimental flag on Nuxt, it seems to call nitro.updateConfig() here.

Then Nitro updateConfig() seems to call normalizeRuntimeConfig here which I assume set the environment to ''

@@ -90,7 +90,7 @@ async function nitroModule(nitro: Nitro) {
// Dual compatibility with Nuxt and Nitro Modules
export default function nitroCloudflareDev(arg1: unknown, arg2: unknown) {
if ((arg2 as Nuxt)?.options?.nitro) {
(arg2 as Nuxt).hooks.hook("nitro:config", (nitroConfig) => {
(arg2 as Nuxt).hooks.hookOnce("nitro:config", (nitroConfig) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some idea: unjs/hookable#105

examples/nuxt/nuxt.config.ts Outdated Show resolved Hide resolved
examples/nuxt/nuxt.config.ts Show resolved Hide resolved
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It is likely an upstream issue, but looks nice hotfix.

Followup: #51

@pi0 pi0 merged commit b39ea5a into main Oct 22, 2024
2 checks passed
@pi0 pi0 deleted the fix/env branch October 22, 2024 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing Cloudflare DB binding (D1)
3 participants