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: externalize node:async_hooks for all adapters #5910

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

wmertens
Copy link
Member

No description provided.

Copy link

netlify bot commented Feb 29, 2024

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit e578b36

@wmertens wmertens enabled auto-merge (rebase) February 29, 2024 08:06
@wmertens wmertens merged commit 1107d41 into QwikDev:main Feb 29, 2024
21 checks passed
@VinSpee
Copy link

VinSpee commented Mar 5, 2024

Hi! After upgrading to 1.5.0, I'm getting this error building for production (vercel adapter if it's helpful)

RollupError: [commonjs--resolver] Cannot bundle Node.js built-in "node:async_hooks" imported from "node_modules/.pnpm/@builder.io+qwik-city@1.5.0_@types+node@20.11.24/node_modules/@builder.io/qwik-city/middleware/request-handler/index.mjs". Consider disabling ssr.noExternal or remove the built-in dependency.

feels related.

@wmertens wmertens deleted the fix-cf-als branch March 5, 2024 15:33
@wmertens
Copy link
Member Author

wmertens commented Mar 5, 2024

Hi all, you need to add node:async_hooks to your externals. I'll add a comment to the release.

@brandonpittman
Copy link
Contributor

Even after adding that to vite.config.ts, I'm still getting this error when building for production.

#5908 (comment)

@wmertens
Copy link
Member Author

wmertens commented Mar 6, 2024

@brandonpittman can you share your configuration? It should be impossible for you to get that error

@brandonpittman
Copy link
Contributor

@wmertens

import { defineConfig } from "vite";
import { qwikVite } from "@builder.io/qwik/optimizer";
import { qwikCity } from "@builder.io/qwik-city/vite";
import tsconfigPaths from "vite-tsconfig-paths";
import { vanillaExtractPlugin } from "styled-vanilla-extract/vite";
import { qwikTypes } from "@builder.io/qwik-labs/vite";

export default defineConfig(() => {
  return {
    define: {
      // enables debugging in workbox
      "process.env.NODE_ENV": JSON.stringify("development"),
    },
    plugins: [
      qwikCity(),
      qwikVite(),
      tsconfigPaths(),
      qwikTypes(),
      vanillaExtractPlugin(),
    ],
    preview: {
      headers: {
        "Cache-Control": "public, max-age=600",
      },
    },
  };
});

Here's my Vite config. Nothing crazy.

@intellix
Copy link
Contributor

intellix commented Mar 6, 2024

the config you've pasted doesn't include the externals change:

export default defineConfig(() => {
  return {
    ssr: {
      external: ["node:async_hooks"],
    },

@wmertens
Copy link
Member Author

wmertens commented Mar 6, 2024

Actually I just realized I can define the externals in the qwikVite plugin 🤦. I'm doing that now and will release qwik@dev with this change shortly.

@wmertens
Copy link
Member Author

wmertens commented Mar 6, 2024

@brandonpittman can you upgrade to qwik@dev? That should fix the production build without further changes

@wmertens
Copy link
Member Author

wmertens commented Mar 6, 2024

Actually it works fine in test projects for me, so I'm deploying 1.5.1.

@brandonpittman
Copy link
Contributor

brandonpittman commented Mar 7, 2024

@wmertens

Ah, it's having a problem in the Cloudflare adapter's vite.config.ts.

npm run dev and npm run preview are fine—just npm run build throws the error. I add the external block to my Cloudflare adapter's vite.config.ts but the error still occurs.

import { cloudflarePagesAdapter } from "@builder.io/qwik-city/adapters/cloudflare-pages/vite";
import { extendConfig } from "@builder.io/qwik-city/vite";
import baseConfig from "../../vite.config";

// @ts-expect-error
export default extendConfig(baseConfig, () => {
  return {
    build: {
      ssr: true,
      rollupOptions: {
        input: ["src/entry.cloudflare-pages.tsx", "@qwik-city-plan"],
      },
    },
    ssr: {
      external: ['node:async_hooks'],
    },
    plugins: [cloudflarePagesAdapter()],
  };
});

@brandonpittman
Copy link
Contributor

brandonpittman commented Mar 7, 2024

@wmertens

I figured out the problem: Vite 4. I started this app before Vite 5 was released and it was working fine up till now. I created a new app and compared the package.json and found that my Vite was v4 while the fresh one was v5.

Probably could use a mention in the release notes.

@wmertens
Copy link
Member Author

wmertens commented Mar 7, 2024

@brandonpittman so you're using 1.5.1 and iets still broken, and you upgraded to vite 5 and it works?

@brandonpittman
Copy link
Contributor

brandonpittman commented Mar 7, 2024

@wmertens

1.5.1 with Vite 4 is broken.

1.5.1 with Vite 5 is fine.

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.

5 participants