Regression bug on node imports

Omikorin commented 11 months ago

Astro Info

Astro                    v4.0.1
Node                     v18.17.0
System                   Linux (x64)
Package Manager          pnpm
Output                   static
Adapter                  none
Integrations             @astrojs/react
                         @pandacss/astro
                         @astrojs/mdx
                         @astrojs/sitemap

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

No response

Describe the Bug

I believe #8996 introduced a regression in these lines in the Code.astro which fails build from v3.4.4 to the latest version.

The best explanation will be with my screenshots of the build (you can check logs here -> Build packages -> @ark-ui/website).

v4.0.1:

Below I include a more clear build log from version v.3.4.4 which points directly to Code.astro

What's the expected result?

Successful build without a crash on node imports.

I added a minimal reproduction example as a branch of affected project because I encountered an issue here so I find it the best playground. When I have some free time, I'll make a minimal reproduction example on Stackblitz if you need.

Link to Minimal Reproducible Example

https://github.com/chakra-ui/ark/tree/build/astro-3.4.4-regression

Participation

[X] I am willing to submit a pull request for this issue.

ematipico commented 11 months ago

@bluwy maybe this is something you might want to look at

bluwy commented 11 months ago

I haven't looked at the repro, but the first few things I'd mention is that:

https://github.com/withastro/astro/pull/8996 has been removed in Astro 4, so it shouldn't be causing the issue.
If https://github.com/withastro/astro/pull/8996 was causing the issue in Astro 3, it's likely that you imported Code.astro in your client code? Could be barrel files causing it to snuck in.
The warnings and errors you get in Astro 4 should also be the same cause, that you're importing node-only code in the client side somewhere.

bluwy commented 11 months ago

The issue is due to this file:

https://github.com/chakra-ui/ark/blob/875023fc5cf5425bdad1d8fd1a085a8fed84e375/packages/website/src/components/docs/navigation/index.ts#L1-L4

The barrel file is exporting both Astro files and TSX files. Which means when you import TableOfContent (TSX component) and hydrate it, the barrel file gets bundled in the client alongside the nodejs dependencies, which it shouldn't get bundled.

On the usage-side in docs-layout.astro, you can fix it with this instead:

import { Footer, Navbar, Sidebar } from '~/components/docs/navigation'
import { TableOfContent } from '~/components/docs/navigation/table-of-content'

Barrel files should be avoided to prevent issues like this in general 😬

Also, I have to run pnpm update rollup to fix https://github.com/rollup/rollup/issues/5259 from happening.

Omikorin commented 11 months ago

@bluwy thank you very much! It works :)

withastro / astro