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

Unable to load your Astro config when linked cjs dependency is imported in astro.config.ts #11621

Closed
1 task
hi-ogawa opened this issue Aug 5, 2024 · 2 comments
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) requires refactor Bug, may take longer as fixing either requires refactors, breaking changes, or considering tradeoffs

Comments

@hi-ogawa
Copy link

hi-ogawa commented Aug 5, 2024

Astro Info

Astro                    v4.13.1
Node                     v20.14.0
System                   Linux (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             none

(note that I'm commenting out cjs import to run `astro info` since otherwise it's breaking cli)

If this issue only occurs in one browser, which browser is a problem?

NA

Describe the Bug

Related: napi-rs/napi-rs#2206

When cjs linked dependency is imported in astro.config.ts, it throws an error since Vite's ssrLoadModule doesn't support cjs global such as require and module.

When such dependency is only used inside app, users can workaround it by explicitly externalizing by vite.ssr.external, but that seems impossible when ssrLoadModule is used when loading astro.config.ts itself.

//// astro.config.ts

import { defineConfig } from 'astro/config';

// @ts-ignore
import testDepCjs from "test-dep-cjs";  // <-- this is linked dep
console.log(testDepCjs);

// https://astro.build/config
export default defineConfig({});
$ pnpm dev

> [email protected] dev /home/hiroshi/code/personal/reproductions/vite-ssr-cjs-astro-napi-rs-2206
> astro dev

12:03:37 PM [vite] Error when evaluating SSR module /fixtures/test-dep-cjs/index.js:
|- ReferenceError: require is not defined
    at /home/hiroshi/code/personal/reproductions/vite-ssr-cjs-astro-napi-rs-2206/fixtures/test-dep-cjs/index.js:2:1
    at instantiateModule (file:///home/hiroshi/code/personal/reproductions/vite-ssr-cjs-astro-napi-rs-2206/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:52650:11)

12:03:37 PM [vite] Error when evaluating SSR module /home/hiroshi/code/personal/reproductions/vite-ssr-cjs-astro-napi-rs-2206/astro.config.ts:
|- ReferenceError: require is not defined
    at /home/hiroshi/code/personal/reproductions/vite-ssr-cjs-astro-napi-rs-2206/fixtures/test-dep-cjs/index.js:2:1
    at instantiateModule (file:///home/hiroshi/code/personal/reproductions/vite-ssr-cjs-astro-napi-rs-2206/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:52650:11)

[astro] Unable to load your Astro config

What's the expected result?

Config loads successfully.

I'm not so familiar with Astro, but when looking at the code, it looks like it's desired/planned to externalize all deps here. Perhaps, this can be switched to ssr.external: true since the linked PR is already merged long time ago.

ssr: {
// NOTE: Vite doesn't externalize linked packages by default. During testing locally,
// these dependencies trip up Vite's dev SSR transform. Awaiting upstream feature:
// https://github.com/vitejs/vite/pull/10939
external: [
'@astrojs/tailwind',
'@astrojs/mdx',
'@astrojs/react',
'@astrojs/preact',
'@astrojs/sitemap',
'@astrojs/markdoc',
'@astrojs/db',
],
},

Link to Minimal Reproducible Example

https://stackblitz.com/github/hi-ogawa/reproductions/tree/main/vite-ssr-cjs-astro-napi-rs-2206?file=README.md

(note: on stackblitz, it looks like ssrFixStacktrace is also crashing so the error log is different. original code can be found in https://github.com/hi-ogawa/reproductions/tree/main/vite-ssr-cjs-astro-napi-rs-2206)

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Aug 5, 2024
@bluwy
Copy link
Member

bluwy commented Aug 5, 2024

Yeah, we're planning to switch to ssr.external: true in the next major as it's a breaking change for existing users who relied on the previous behaviour.

@bluwy bluwy added - P3: minor bug An edge case that only affects very specific usage (priority) requires refactor Bug, may take longer as fixing either requires refactors, breaking changes, or considering tradeoffs and removed needs triage Issue needs to be triaged labels Aug 5, 2024
@florian-lefebvre
Copy link
Member

florian-lefebvre commented Oct 5, 2024

Closing as the change has been made on the next branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) requires refactor Bug, may take longer as fixing either requires refactors, breaking changes, or considering tradeoffs
Projects
None yet
Development

No branches or pull requests

3 participants