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: make vike-cloudflare a real Vike extension (#37) #39

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

brillout
Copy link
Member

No description provided.

Copy link

cloudflare-workers-and-pages bot commented Jan 23, 2025

Deploying vike-cloudflare-vike-demo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0abdcc3
Status: ✅  Deploy successful!
Preview URL: https://526ab4f7.vike-clouflare-vike-demo.pages.dev
Branch Preview URL: https://brillout-dev.vike-clouflare-vike-demo.pages.dev

View logs

Copy link

cloudflare-workers-and-pages bot commented Jan 23, 2025

Deploying vike-cloudflare-hono-demo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0abdcc3
Status: ✅  Deploy successful!
Preview URL: https://099a1c6f.vike-clouflare-hono-demo.pages.dev
Branch Preview URL: https://brillout-dev.vike-clouflare-hono-demo.pages.dev

View logs

Copy link

cloudflare-workers-and-pages bot commented Jan 23, 2025

Deploying vike-cloudflare-hattip-demo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0abdcc3
Status: ✅  Deploy successful!
Preview URL: https://3169a664.vike-clouflare-hattip-demo.pages.dev
Branch Preview URL: https://brillout-dev.vike-clouflare-hattip-demo.pages.dev

View logs

@brillout
Copy link
Member Author

The test examples/vike-app/.test-preview.test.ts seems to fail only because wrangler logs something on stderr:

FAIL  /home/runner/work/vike-cloudflare/vike-cloudflare/examples/vike-app/.test-preview.test.ts
Test  FAIL  because error(s)/warning(s) occurred during server termination, see below. [TEST_FAIL]
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvv ERROR & WARNING LOGS vvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
[20:19:17.[137](https://github.com/vikejs/vike-cloudflare/actions/runs/12937347304/job/36085068099?pr=39#step:8:138)][/examples/vike-app][pnpm run preview --port 3000][stderr] 🪵  Logs were written to "/home/runner/.config/.wrangler/logs/wrangler-2025-01-23_20-19-14_233.log"

I wonder why it does this only in this PR, not in main. I can't think of a change introduced by this PR that would explain this log.

@brillout
Copy link
Member Author

I found why: it's because Vike logs a new warning on stderr. It seems that if something is logged on stderr then wrangler logs Logs were written to ... on stderr.

@brillout
Copy link
Member Author

The warning is this:

[vike][Warning] The environment is set to be a development environment by process.env.NODE_ENV===null, which seems contradictory because the environment seems to be a production environment (Vite isn't loaded), see https://vike.dev/NODE_ENV

Removing that warning makes the CI green for examples/vike-app/.

How can we get rid of that warning? Ideas on why process.env.NODE_ENV isn't 'production'?

See also:

It's surprising that it's 'production' at packages/vike-cloudflare/assets/vike.js but null inside Vike's code. Maybe it's a Vite misconfiguration on vike-cloudflare's side?

@brillout
Copy link
Member Author

It's surprising that it's 'production' at packages/vike-cloudflare/assets/vike.js but null inside Vike's code. Maybe it's a Vite misconfiguration on vike-cloudflare's side?

The warning code isn't inside dist/ so this doesn't seem to be the issue. It does seem to be a Wrangler issue after all.

@brillout
Copy link
Member Author

The examples/vike-app/ is now green. I'll have a look at why the other ones are red later.

BREAKING CHANGE: update vike to `>=0.4.219`
@brillout
Copy link
Member Author

It's only examples/hattip-app/.test-preview.test.ts that is red, the rest is green. I suspect it's something to do with @hattip/vite.

@magne4000
Copy link
Member

It's only examples/hattip-app/.test-preview.test.ts that is red, the rest is green. I suspect it's something to do with @hattip/vite.

I found a fix/workaround. Not perfect, but at least it seems to work

@magne4000
Copy link
Member

@brillout still a draft or this can be merged?

@brillout
Copy link
Member Author

I think it's best I finish my refactoring endeavour first. I'll keep you updated — ETA this week.

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.

2 participants