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

Bypass the worker to serve static pages #138

Open
DevsDidSomething opened this issue Nov 20, 2024 · 2 comments
Open

Bypass the worker to serve static pages #138

DevsDidSomething opened this issue Nov 20, 2024 · 2 comments
Labels

Comments

@DevsDidSomething
Copy link

Describe the bug

Pre-rendered static pages (e.g., /helloWorld) are being incorrectly served from /cdn-cgi/_cf_seed_data instead of the /assets folder. This causes unnecessary worker invocations and CPU spikes of 200-300ms per page visit.

Fix would require moving the content inside of "/cdn-cgi/_cf_seed_data" to the root of the "/assets" folder. I manually moved a test "test.html" into my assets folder and can confirm that visiting "/test" on my deployed website doesn't hit my worker which is the expectation.

image

Steps to reproduce

  1. Create default nextjs project for workers.
  2. Deploy
  3. Run tail worker
  4. Expectation is that visiting the static home page should not be hitting your tail worker

Expected behavior

Pages pre-rendered as static should not hit your worker
Static content should be served directly from /assets

@opennextjs/cloudflare version

0.2.1

Node.js version

22.1

Wrangler version

3.88

next info output

Operating System:
  Platform: darwin
  Arch: arm64
Binaries:
  Node: 22.1.0
  npm: 10.7.0
  Yarn: 1.22.19
  pnpm: 8.4.0
Relevant Packages:
  next: 14.2.5 // An outdated version detected (latest is 15.0.3), upgrade is highly recommended!
  eslint-config-next: 14.2.5
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.6.3
Next.js Config:
  output: N/A

Additional context

No response

@DevsDidSomething DevsDidSomething added bug Something isn't working triage labels Nov 20, 2024
@vicb vicb added enhancement New feature or request needs investigation and removed bug Something isn't working triage labels Nov 21, 2024
@vicb vicb changed the title [BUG] Worker hit on static assets Worker hit on static assets Nov 21, 2024
@DevsDidSomething
Copy link
Author

Played around with opennext, and I was able to fix this on main.

I made a modification to the incremental-cache.ts file and made export const SEED_DATA_DIR = "" and pre-rendered pages no longer hit my worker.

This only works on the main branch, i've noticed that on the experimental middleware branch, it'll still hit my worker (I assume because the middleware is being called).

@vicb vicb changed the title Worker hit on static assets Bypass the worker to serve static pages Nov 22, 2024
@vicb
Copy link
Contributor

vicb commented Nov 22, 2024

Played around with opennext, and I was able to fix this on main.

I would not say "fix" here because:

  • while the page content is static, I believe you should still apply the middleware and the rewrites/headers in the next config
  • Setting SEED_DATA_DIR to "" will expose the cache content on public URLs - it's ok for static content but that's probably not something you want for i.e. fetch cache that might include private info/secrets.

Also as you noted the experimental branch is different for now. Cache is now ready there yet.

Once the experimental branch becomes the main branch we can revisit that. It might be possible to determine that a static page would not be affected by the routing (middleware + headers/rewrites) at build time. In such a case we could serve the page without hitting the worker. In other cases we will still need to run the middleware/routing layer.

Note: what OpenNext refers to as "middleware" is actually Next routing + middleware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants