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

Builds fail when MDX souremaps are enabled on files with import.meta.env.* #9012

Closed
1 task done
nerdoza opened this issue Nov 6, 2023 · 3 comments · Fixed by #9652
Closed
1 task done

Builds fail when MDX souremaps are enabled on files with import.meta.env.* #9012

nerdoza opened this issue Nov 6, 2023 · 3 comments · Fixed by #9652
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@nerdoza
Copy link
Contributor

nerdoza commented Nov 6, 2023

Astro Info

Astro                    v3.4.3
Node                     v18.18.2
System                   Windows (x64)
Package Manager          npm
Output                   server
Adapter                  astro-sst
Integrations             @astrojs/mdx
                         @astrojs/sitemap
                         @astrojs/prefetch
                         astro-robots-txt

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

n/a

Describe the Bug

When enabling sourcemaps in the vite.build configuration with the MDX integration, the build will fail if any of the .mdx files references environment variables.

Example MDX Files:

---
title: 'Using ENVs'
description: 'Lorem ipsum dolor sit amet'
pubDate: 'Oct 25 2023'
heroImage: '/img/placeholder-hero.jpg'
---

export const SECRET_KEY = import.meta.env.SECRET_KEY
export const VITE_KEY = import.meta.env.VITE_KEY
export const PUBLIC_KEY = import.meta.env.PUBLIC_KEY

Let's see what environment variables are available to us:

- `SECRET_KEY`: { SECRET_KEY }
- `VITE_KEY`: { VITE_KEY }
- `PUBLIC_KEY`: { PUBLIC_KEY }

Resulting error:

> @example/[email protected] dev
> astro dev

  🚀  astro  v3.4.3 started in 300ms
  
  ┃ Local    http://localhost:4321/
  ┃ Network  use --host to expose
  
02:18:22 PM [astro] update /src/pages/test.mdx (x6)
TypeError: Cannot destructure property 'length' of 'code' as it is undefined.
    at State.map (file:///home/projects/github-jmjzta/node_modules/astring/dist/astring.mjs:1166:13)
    at State.writeAndMap (file:///home/projects/github-jmjzta/node_modules/astring/dist/astring.mjs:1123:10)
    at Object.Literal (file:///home/projects/github-jmjzta/node_modules/astring/dist/astring.mjs:1057:13)
    at Object.VariableDeclarator (file:///home/projects/github-jmjzta/node_modules/astring/dist/astring.mjs:530:27)
    at formatVariableDeclaration (file:///home/projects/github-jmjzta/node_modules/astring/dist/astring.mjs:253:15)
    at Object.VariableDeclaration (file:///home/projects/github-jmjzta/node_modules/astring/dist/astring.mjs:523:5)
    at Object.ExportNamedDeclaration (file:///home/projects/github-jmjzta/node_modules/astring/dist/astring.mjs:624:34)
    at Object.Program (file:///home/projects/github-jmjzta/node_modules/astring/dist/astring.mjs:286:27)
    at generate (file:///home/projects/github-jmjzta/node_modules/astring/dist/astring.mjs:1203:29)
    at Module.toJs (file:///home/projects/github-jmjzta/node_modules/estree-util-to-js/lib/index.js:126:21)

Tracking the issue back up the stack it appears to be originating from the recma-stringify function which is several layers of wrapper around the astring library, so I suspect that's where the issue actually resides.

Looking at this section of code in the astring library, I believe that only import( and import syntaxes are supported.

Path Forward

I'm not sure the proper path forward on this sort of issue. I suspect this has been solved by other libraries so would switching libraries be easier?

What's the expected result?

Be able to build an Astro site with sourcemaps enabled while still using the full power of MDX.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-jmjzta?file=astro.config.mjs

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 Nov 6, 2023
@nerdoza nerdoza changed the title Builds fail when MDX souremaps are enabled on files with import.meta.env.* Builds fail when MDX souremaps are enabled on files with import.meta.env.* Nov 6, 2023
@nerdoza
Copy link
Contributor Author

nerdoza commented Nov 9, 2023

I opened an issue with astring thinking it might be an issue in that library but their dev's confirmed it works as expected with MDX in the playground, so the issue is likely in the implementation within Astro's MDX plugin.

Ref: davidbonnet/astring#700

@nerdoza
Copy link
Contributor Author

nerdoza commented Nov 9, 2023

Upon further testing (based on the hunch provided by @wooorm) I believe the issue is specifically import.meta calls which aren't replaced in the MDX file because they don't match an environment argument.

When I switch from VITE_TEST_VALUE to DEV for the import.meta.env property it compiles fine.

# This breaks.

export const VITE_TEST_VALUE = import.meta.env.VITE_TEST_VALUE;

Environment Value: { VITE_TEST_VALUE }
# This works fine.

export const IS_DEV= import.meta.env.DEV;

Dev Value: { IS_DEV }

I would expect that unmatched values are converted to undefined or null instead of leaving them as an unsupported node in the AST.

@matthewp
Copy link
Contributor

Thanks for breaking this down! If you're able to do a PR that would be appreciated.

@matthewp matthewp added - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged labels Nov 17, 2023
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)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants