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

[Beta] Astro MDX renderer is not bundled #12029

Closed
1 task
universse opened this issue Sep 19, 2024 · 6 comments
Closed
1 task

[Beta] Astro MDX renderer is not bundled #12029

universse opened this issue Sep 19, 2024 · 6 comments
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: container Related to the container API (scope)

Comments

@universse
Copy link

universse commented Sep 19, 2024

Astro Info

Astro                    v5.0.0-beta.1
Node                     v22.6.0
System                   Windows (x64)
Package Manager          pnpm
Output                   static
Adapter                  @astrojs/cloudflare
Integrations             @astrojs/react
                         @astrojs/mdx
                         @astrojs/sitemap

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

No response

Describe the Bug

Deployment to CF Pages failed due to non-existent '@astrojs/mdx/server.js'.

Here's the output of dist\_worker.js\chunks\_@astro-renderers_[hash].mjs.

globalThis.process ??= {}; globalThis.process.env ??= {};
import { p as getDefaultExportFromCjs } from './astro/server_CH__8xxn.mjs';
import _renderer1 from '@astrojs/mdx/server.js'; // <--- here

const opts = {
  experimentalReactChildren: false
};

var react = {exports: {}};
...

What's the expected result?

MDX renderer should be bundled.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-jkjbbg

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 Sep 19, 2024
@bluwy
Copy link
Member

bluwy commented Sep 19, 2024

I can't reproduce the issue with the link given. I'm seeing that it's properly bundled. Perhaps you could double check that all Astro dependencies are upgraded?

@bluwy bluwy added the needs response Issue needs response from OP label Sep 19, 2024
@universse
Copy link
Author

universse commented Sep 19, 2024

I've figured out the issue. I'm using Astro Container API. To make it work on Cloudflare, I previously needed to mark container modules as external and this does not work for Astro v5 beta.

  vite: {
    ssr: {
      // previously v4
      // external: ["astro/container", "@astrojs/mdx"],
      external: [
        'astro/container',
        'node:fs',
        'node:fs/promises',
        'fs',
        'node:url',
        'node:path',
        'path',
      ],
    },
  },

Here's the updated reproduction https://stackblitz.com/edit/github-jkjbbg-nhzsfm

Perhaps it's an issue with Astro Container API? Think it should work out of the box with CF integration.

@matthewp matthewp added - P3: minor bug An edge case that only affects very specific usage (priority) feat: container Related to the container API (scope) and removed needs triage Issue needs to be triaged needs response Issue needs response from OP labels Sep 20, 2024
@ematipico
Copy link
Member

ematipico commented Oct 7, 2024

In order to fix the issue, you need to load the render manually, otherwise it isn't bundled. The way you're loading the renderer is meant for cases like vitest. This is mentioned in the docs:

When using vite (vitest, Astro integrations, etc.), the renderers are loaded with the function loadRenderers() from the virtual module astro:container. Outside vite, you’ll have to load the renderers manually.

Here's how you load the MDX renderers:

import mdxRenderer from "@astrojs/mdx/server.js";

const astroContainer = await AstroContainer.create();
astroContainer.addServerRenderer({renderer: mdxRenderer});

@ematipico ematipico closed this as not planned Won't fix, can't repro, duplicate, stale Oct 7, 2024
@universse
Copy link
Author

universse commented Oct 9, 2024

Not sure I understand correctly. The reason I'm loading it as such is because of Node.js built-in imports causing Vite to throw error when building for Cloudflare workers (via @astrojs/cloudflare integration). Loading the renderers manually does not help.

@ematipico
Copy link
Member

Not sure I understand correctly. The reason I'm loading it as such is because of Node.js built-in imports causing Vite to throw error when building for Cloudflare workers (via @astrojs/cloudflare integration). Loading the renderers manually does not help.

I did test it using your reproduction, and it built successfully.

@universse
Copy link
Author

universse commented Oct 9, 2024

Yes, it built successfully because I added all Node.js built-ins to vite.external.ssr to Astro config. If you comment those out, the build will fail because Astro Container feature depends on Node built-ins. The page using the Astro Container is /blog/[...slug].

What I'm saying is Astro Container API should work with CF integration without having to configure vite.external.ssr. Having vite.external.ssr works fine for prerendered pages but not for server rendered pages that use the Container API.

Issue is somewhat similar to this #6537

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) feat: container Related to the container API (scope)
Projects
None yet
Development

No branches or pull requests

4 participants