withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
47.05k stars 2.5k forks source link

Issue with cssesc import in Transition.ts causing compatibility problems with postcss-preset-env #9880

Closed nick-noaa closed 9 months ago

nick-noaa commented 10 months ago

Astro Info

Astro                    v4.2.7
Node                     v20.5.1
System                   Linux (x64)
Package Manager          bun
Output                   static
Adapter                  none
Integrations             @astrojs/react
                         @astrojs/svelte

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

No response

Describe the Bug

Hello,

I've encountered an issue with Astro version 4.2.5 (and possibly later versions) where the removal of the import cssesc from '/node_modules/cssesc/cssesc.js?v=8e4d546f'; statement in Transition.ts is causing compatibility problems with the postcss-preset-env package.

As a temporary workaround, I've downgraded to Astro 4.2.4.

Could you please look into this issue? Thank you.

What's the expected result?

Astro package to be compatible with postcss-preset-env without requiring manual modifications to the node_modules directory, finding an alternative postcss package or downgrading Astro to 4.2.4.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/withastro-astro-g3bsbv

Participation

natemoo-re commented 10 months ago

Could you please share more information about what the compatability issue you're experiencing is? I'm unable to reproduce this and will need more details to help.

You should not have to modify anything inside of node_modules for Astro to work with another package. Astro now relies on the cssesc package to properly escape the transition:name directive.

I'd also try running this on Node and seeing if you have the same problem? Despite Bun's claims of Node compatibility, they are in fact using different runtime heuristics that could cause issues.

dreadjr commented 10 months ago

I am seeing a similar error (using node) only on 1 page of my project that uses a client side script to utilize @antv/g6.

Astro                    v4.2.7
Node                     v18.17.1
System                   macOS (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             @astrojs/tailwind
                         @astrojs/sitemap
                         @astrojs/mdx
                         astro-icon
                         @astrojs/partytown
                         AstroWind:tasks
                         astro-compress
                         astro-critters

Console errors.

Uncaught SyntaxError: The requested module '/node_modules/cssesc/cssesc.js?v=e7df1019' does not provide an export named 'default' (at transition.js?v=e7df1019:3:8)

If i revert back to previous version of astro, it works again.

This reproduces it for me.

---

---

<script>
  import { getCollection } from 'astro:content';

  const posts = await getCollection('post');
  console.log(posts);
</script>

<div class="container w-full h-full">
  <div id="container">test</div>
</div>
nick-noaa commented 10 months ago

Yes, same with NPM and PNPM. I am getting the same error message as @dreadjr. Also, a lot of my JS (React, Svelte) isn't working. I believe it's just my components with animations.

pijusz commented 10 months ago

Same issue, had to downgrade to 4.2.4. Using: Transitions, React. React does not load at all.

florian-lefebvre commented 10 months ago

Regression introduced in https://github.com/withastro/astro/commit/bd880e8437ea2df16f322f604865c1148a9fd4cf

florian-lefebvre commented 10 months ago

That's because cssesc is shipped as CJS

florian-lefebvre commented 10 months ago

Okay actually I'm not so sure, I can't reproduce it using the repro provided or even in a fork that uses transition names (since I suspect it comes from that part): https://stackblitz.com/edit/withastro-astro-l5ze82?file=src%2Fpages%2Findex.astro. Can someone provide another minimal repro so that we can try to isolate the issue origin? Thanks a lot!

nick-noaa commented 9 months ago

Do we have a update on this? I am trying to reproduce the issue on this https://stackblitz.com/edit/withastro-astro-v7naeu?file=src%2Fpages%2Findex.astro.

icouldbreathe commented 9 months ago
Astro                    v4.3.3
Node                     v18.17.1
System                   macOS (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             @astrojs/tailwind
                         @astrojs/react
                         @astrojs/sitemap
                         auto-import
                         astro-embed
                         @astrojs/mdx

I am getting the same error:

Uncaught (in promise) SyntaxError: The requested module '/node_modules/cssesc/cssesc.js?v=f4474253' does not provide an export named 'default' (at transition.js:3:8)

deps:

"dependencies": {
    "@astrojs/check": "^0.4.1",
    "@astrojs/mdx": "^2.1.1",
    "@astrojs/react": "^3.0.9",
    "@astrojs/rss": "^4.0.5",
    "@fontsource-variable/inter": "^5.0.16",
    "@fontsource-variable/jetbrains-mono": "^5.0.19",
    "@resvg/resvg-js": "^2.6.0",
    "@types/mdast": "^4.0.3",
    "astro": "4.3.3",
    "astro-embed": "^0.6.1",
    "astro-seo-schema": "^4.0.0",
    "fuse.js": "^7.0.0",
    "github-slugger": "^2.0.0",
    "mdast-util-to-string": "^4.0.0",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "reading-time": "^1.5.0",
    "rehype": "^13.0.1",
    "rehype-figure-for-img": "^0.0.6",
    "rehype-pretty-code": "^0.12.6",
    "remark-oembed": "^1.2.2",
    "remark-rehype": "^11.1.0",
    "satori": "^0.10.13",
    "schema-dts": "^1.1.2",
    "shikiji": "^0.10.2",
    "tailwindcss": "^3.4.1",
    "typescript": "^5.3.3",
    "unist-util-visit": "^5.0.0"
  },
  "devDependencies": {
    "@astrojs/sitemap": "^3.0.5",
    "@astrojs/tailwind": "^5.1.0",
    "@astropub/md": "^0.4.0",
    "@divriots/jampack": "^0.23.3",
    "@tailwindcss/typography": "^0.5.10",
    "@types/github-slugger": "^1.3.0",
    "@types/node": "^20.11.16",
    "@types/react": "^18.2.55",
    "@types/remark-oembed": "^1.2.4",
    "@typescript-eslint/parser": "^6.21.0",
    "astro-eslint-parser": "^0.16.3",
    "commitizen": "^4.3.0",
    "cz-conventional-changelog": "^3.3.0",
    "eslint": "^8.56.0",
    "eslint-plugin-astro": "^0.31.4",
    "husky": "^9.0.10",
    "lint-staged": "^15.2.2",
    "prettier": "^3.2.5",
    "prettier-plugin-astro": "^0.13.0",
    "prettier-plugin-tailwindcss": "^0.5.11"
  },
  "peerDependencies": {
    "react": "^18.2.0",
    "react-dom": "^18.2.0"
  },

I can reproduce it by just using a react component with client:load. I am not using any ViewTransitions.

pages/test.astro:

---
import TestComp from "@components/views/Test";
import { getCollection } from "astro:content";

const posts = await getCollection("posts");

---
<ul>
    {posts && (
        posts.map(( post ) => (
            <TestComp client:load post={post} />
        ))
    )}
</ul>

components/test.tsx:

import { type CollectionEntry } from "astro:content";

export interface Props {
    post: CollectionEntry<"posts">;
}

export default function TestComp({post}: Props) {
    return (
        <li>{post.data.title}</li>
    );
}
dreadjr commented 9 months ago

https://stackblitz.com/edit/withastro-astro-wbx6n3?file=src%2Fpages%2Findex.astro

Look at browser console logs

bluwy commented 9 months ago

The core issue here is that astro:content is being imported on the client-side. I'm not really sure if this is intended to work even if the build+preview not fail because it bundled fine. However, if you inspect the built output, you'll notice a lot of unrelated JS code bundled together.

It happens to work because the Astro server runtime is environment agnostic (so it works in Deno, Cloudflare workers, etc), but it also happens to work in browsers. I wonder if we should only allow importing astro:content in SSR to prevent this. It's kinda a breaking change I suppose, but prevents mis-using in the client.

dreadjr commented 9 months ago

The core issue here is that astro:content is being imported on the client-side. I'm not really sure if this is intended to work even if the build+preview not fail because it bundled fine. However, if you inspect the built output, you'll notice a lot of unrelated JS code bundled together.

It happens to work because the Astro server runtime is environment agnostic (so it works in Deno, Cloudflare workers, etc), but it also happens to work in browsers. I wonder if we should only allow importing astro:content in SSR to prevent this. It's kinda a breaking change I suppose, but prevents mis-using in the client.

The error is the original exception from the PR. This is just a quick way to reproduce it. Others and myself get the error under more complex and "valid" code.

bluwy commented 9 months ago

Well we have to see how the complex scenarios could look like to further determine the issue 😅 I'm seeing only a single repro so far that shows how the error can be introduced.

nick-noaa commented 9 months ago

Well we have to see how the complex scenarios could look like to further determine the issue 😅 I'm seeing only a single repro so far that shows how the error can be introduced.

Thank you for your response! I understand that diving into complex scenarios can shed more light on the issue. Unfortunately, since my repository is private, I've been attempting to manually recreate the problem on StackBlitz. Here's the link to my attempt: StackBlitz Reproduction.

Initially, I suspected the issue might be related to PostCSS-preset-env, but after further investigation, I'm no longer certain. It's worth noting that other users experiencing similar problems are also utilizing both React and Svelte in their projects, alongside tools like Framer Motion. Any insights or suggestions would be greatly appreciated. Thanks again for your assistance!

lilnasy commented 9 months ago

The error is the original exception from the PR. This is just a quick way to reproduce it.

Thanks for looking into it! What you have found out suggests the current error is a symptom of a serious bug, rather the bug itself. We can fix things around cssesc (and one attempt is available to try in #10006), but the underlying problem of server code being sent to the browser remains.

We would need some help from those who are running into this to get to the root cause.

slaetthult commented 9 months ago

I have a similar issue. We don't use ViewTransitions. Since Astro 4.2.5 I get errors if we use:

    adapter: netlify({
        edgeMiddleware: true
    })

edgeMiddleware: false prevents the errors, but we depend on it. Bildschirmfoto 2024-02-08 um 15 45 48

lilnasy commented 9 months ago

The workaround for netlify edge middleware is this:

code node_modules/astro/dist/runtime/server/transition.js
-import cssesc from "cssesc";
+import cssesc from "cssesc/cssesc.js";

This applies only to Netlify Edge Middleware users. There are two manifestations of cssesc being improperly packaged - the browser error suggests an underlying issue, while the netlify error is safe to workaround.

Edit: I should add that I was deploying through the netlify CLI, you can't edit node modules when the project is being built by Netlify's git-based CI.

bluwy commented 9 months ago

@icouldbreathe re https://github.com/withastro/astro/issues/9880#issuecomment-1930588983, you need to use import type { CollectionEntry } from "astro:content"; instead. The type inside the braces means that astro:content would still be bundled in the client do to side-effects, because when compiled, it becomes import {} from "astro:content" in JS.

icouldbreathe commented 9 months ago

@icouldbreathe re #9880 (comment), you need to use import type { CollectionEntry } from "astro:content"; instead. The type inside the braces means that astro:content would still be bundled in the client do to side-effects, because when compiled, it becomes import {} from "astro:content" in JS.

Did not know that! Thanks, it fixed the issue for me.

nick-noaa commented 9 months ago

@icouldbreathe re #9880 (comment), you need to use import type { CollectionEntry } from "astro:content"; instead. The type inside the braces means that astro:content would still be bundled in the client do to side-effects, because when compiled, it becomes import {} from "astro:content" in JS.

Did not know that! Thanks, it fixed the issue for me.

This looks to be narrowing it down at least 🙂

I am using export async function getStaticPaths() to pull dynamic data from a Drupal API.

I don't know if it's related but I am also getting a bunch of 404 errors from images [404] /%22/sites/g/files/anmtlf171/files/styles/...

icouldbreathe commented 9 months ago

I don't know if it's related but I am also getting a bunch of 404 errors from images [404] /%22/sites/g/files/anmtlf171/files/styles/...

Actually I wonder if it could be related, because I did see these errors myself, but only using my test repro code. Although the errors still appear despite moving type out of the curly brackets.

Only seem to happen for assets that were included in my .md files via <img> or <video> tags, and not with markdown syntax ![]().

It can't find these assets because for some reason %22 is attached to the beginning and end of the path, which just represents the character ".

I have no idea, maybe it's a different issue?

nick-noaa commented 9 months ago

I don't know if it's related but I am also getting a bunch of 404 errors from images [404] /%22/sites/g/files/anmtlf171/files/styles/...

Actually I wonder if it could be related, because I did see these errors myself, but only using my test repro code. Although the errors still appear despite moving type out of the curly brackets.

It seems that these 404 errors are specific to the development environment and aren't present on the live site.

Upon inspecting, I found that they're embedded within Astro's new DevToolbar. It appears that these errors originate from within the toolbar itself and may not be directly linked to this issue.

nick-noaa commented 9 months ago

I discovered the issue on my end. There was an unused import statement, import { getImage } from 'astro:assets';, in my imageUtils.ts file. I had added it a while back when I was trying to create a utility function for formatting dynamic images, but then forgot to remove it.

However, this doesn't resolve the 404 errors I mentioned previously...

I presume that any import from 'astro:assets' in JavaScript could potentially cause this issue?