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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 BUG: 'fileURLToPath' is not exported by __vite-browser-external, imported by node_modules/@astrojs/image/dist/build/ssg.js #4101

Closed
1 task
annaahsu opened this issue Jul 30, 2022 · 15 comments 路 Fixed by #4330
Assignees
Labels
- P2: has workaround Bug, but has workaround (priority)

Comments

@annaahsu
Copy link

annaahsu commented Jul 30, 2022

What version of astro are you using?

1.0.0-rc

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

yarn

What operating system are you using?

Mac

Describe the Bug

I recently attempted to upgrade to 1.0.0-rc for both MDX and <Picture> support. I attempted to push the branch to Cloudflare (just SSG not SSR), which was unsuccessful albeit functional locally. The log file from Cloudflare is attached here.

I'm not sure if the attached CodeSandbox is going to be any use, since it looks like it's either having different problems or not having a problem at all, so here's the branch I'm having issue with.

Link to Minimal Reproducible Example

https://codesandbox.io/s/annahsu-go7045?file=/README.md

EDIT: The issue is specific to images in MDX and can be reproduced locally with the original repo's mdx branch here

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added this to Needs Triage in 馃悰 Bug Tracker Jul 30, 2022
@FredKSchott
Copy link
Member

@annaahsu can you confirm that you're on the latest release of astro and @astrojs/image? We believe that this was fixed in the last release! cc @tony-sull to confirm

@FredKSchott FredKSchott added the needs response Issue needs response from OP label Aug 1, 2022
@annaahsu
Copy link
Author

annaahsu commented Aug 1, 2022

I'm on astro version 1.0.0-rc.3 and @astrojs/image version 0.3.0. I deployed on Cloudflare with NODE_VERSION of v16.12.0. The issue is still there鈥攄o you think it'd be a problem with my code instead of Astro's? The only thing I changed image-wise was updating an image to be <Picture> instead of <Image>.

@FredKSchott FredKSchott removed the needs response Issue needs response from OP label Aug 2, 2022
@tony-sull tony-sull moved this from Needs Triage to Accepted in 馃悰 Bug Tracker Aug 3, 2022
@tony-sull
Copy link
Contributor

Ah interesting, I think it may related to a change last week that moved the node imports to use node:url instead of url

I'm not as familiar with some of the more detailed differences in Cloudflare's build environment, interesting that this seems to be a Cloudflare-specific issue 馃

@tony-sull tony-sull added - P4: important Violate documented behavior or significantly improves performance (priority) s1-small labels Aug 3, 2022
@annaahsu
Copy link
Author

annaahsu commented Aug 3, 2022

I just tried deploying to Netlify and it looks like it's having the same problem.

@tony-sull
Copy link
Contributor

Ah ha, looks like this is actually specific to the images being used in MDX. I'm not as familiar with exactly how MDX is tied in under the covers but will loop in @bholmesdev, our resident MDX expert 馃殌


Turns out the CodeSandbox link doens't reproduce the issue in Cloudflare, but that was super helpful as a sanity check that images should work in Cloudflare Pages builds! I cloned the original project's mdx branch and was able to reproduce this locally (build error goes away when removing the <Image /> components in .mdx files)

@tony-sull
Copy link
Contributor

Notes from checking with Fred - this may actually be related to an issue where [rollup-plugin-polyfill-node](https://github.com/FredKSchott/rollup-plugin-polyfill-node) doesn't include a polyfill for url 馃檭

Todo - test locally to see if using the latest fork linked above fixes this, if so that may just mean Vite needs a dependency version bump to pick up the fix

@tony-sull tony-sull added - P3: minor bug An edge case that only affects very specific usage (priority) and removed - P4: important Violate documented behavior or significantly improves performance (priority) labels Aug 3, 2022
@rkuprella
Copy link

rkuprella commented Aug 8, 2022

Same issue here, but I'm not using Image or Picture inside of mdx files. Everything used to work until yesterday morning.

edit: I solved my issue. Apparently I had client:load on an astro component that included my image component. Removing it fixed it.

@FredKSchott FredKSchott added - P4: important Violate documented behavior or significantly improves performance (priority) and removed - P3: minor bug An edge case that only affects very specific usage (priority) labels Aug 8, 2022
@bluwy bluwy self-assigned this Aug 12, 2022
@bluwy
Copy link
Member

bluwy commented Aug 15, 2022

Thanks for the hint @rkuprella. Indeed it looks like the case here too. BlogImage (from blogimage.astro) is being created with client:visible, which isn't valid for an astro file. There should be a warning during dev that should warn about this, but perhaps using MDX that it's not properly surfaced.

Removing all the client:visible in the repro fixed it for me.

@bluwy bluwy added - P2: has workaround Bug, but has workaround (priority) and removed - P4: important Violate documented behavior or significantly improves performance (priority) labels Aug 15, 2022
@bluwy
Copy link
Member

bluwy commented Aug 15, 2022

Tried to take a stab at this for a warning. So it seems like because @astro/mdx compiles into JSX, we don't have warnings setup as usually JSX is considered a UI framework (?), instead of templating. I made a branch here, but I'm not sure if this is the right way to do so. I know Astro would usually detect these warnings in runtime instead. I might need some help from @bholmesdev

@bholmesdev
Copy link
Contributor

@bluwy Good find! Yes, you're exactly right about how MDX is handled. We leave jSX un-transformed by our Vite plugin, and let Astro's JSX runtime swoop in to compile. I showed that branch to @natemoo-re and it sounds like your fix is worth PR-ing!

@gannonh
Copy link

gannonh commented Dec 19, 2022

I've having this same issue:

    "@astrojs/image": "^0.12.1",
    "@astrojs/mdx": "^0.13.0",
    "astro": "^1.6.13",

Any ideas?

@gannonh
Copy link

gannonh commented Dec 19, 2022

Btw, the error is thrown at build time just from having an import statement in an astro component. Not evening using the Picture component.

import { Picture } from "@astrojs/image/components";

@sergeylukin
Copy link

sergeylukin commented Mar 18, 2023

It happened to me too this morning. Spent like 3 hours investigating. The reason was that I have mixed Astro's content collection function inside client side React .tsx module. It was not obvious as the actual getCollection() call

// react-component.tsx
import React from 'react';
import { foo } from '~/foo'; // it's ok, regular .ts file

const MyComponent = () => {
   ...
  foo(); // seems fine so far
  ...
}

// foo.ts
import { bar } from 'bar'; // now that becomes dangerous...
...
export const foo = () => bar(); // damn...

// bar.ts
import { getCollection } from 'astro:content'; // oh-oh..
...

export const bar() => {
  ...
  getCollection('posts'); //BAAAM!! that causes the error, don't make SSR-island cocktails 馃嵏 
  ...
}

was down the import tree so it's tricky to find it. Hope that helps. I guess that's the price for the freedom and cross-compatibility Astro provides :) I still love Astro very much.

@bholmesdev
Copy link
Contributor

Super helpful example @sergeylukin. Speaks to a need to detect server-side deps like Node with a clear error message. We're also trying to update our packages to strip out Node where possible.

@sergeylukin
Copy link

sergeylukin commented Mar 18, 2023

@bholmesdev I agree. It's a trap that is easy to get into. Would be great to have a human-readable warning sign here. Smth like "Oops...client side island X depends on server side module Y. Can't proceed. Please resolve" would be amazing 馃帀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: has workaround Bug, but has workaround (priority)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

8 participants