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

Using MDX and building for production outputs both optimized and original image assets #10066

Closed
1 task done
ddotlic opened this issue Feb 9, 2024 · 3 comments · Fixed by #10278
Closed
1 task done
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: assets Related to the Assets feature (scope) pkg: mdx Issues pertaining to `@astrojs/mdx` integration

Comments

@ddotlic
Copy link

ddotlic commented Feb 9, 2024

What version of starlight are you using?

0.17.1

What version of astro are you using?

4.2.6

What package manager are you using?

yarn 1.x

What operating system are you using?

macOS 14.3

What browser are you using?

Not browser specific

Describe the Bug

When the documentation is in "plain" Markdown files .md then the images get optimized and copied out into dist/_astro.

As soon as we renamed all the documentation files to .mdx (was a necessity, needed to use Astro/Starlight components), building for production yields both optimized and original assets. Specifically, am talking about images placed in src/assets (those in public are IIUC not optimized anyway).

To repro, simply create a new project, add a .png image to assets and reference it from a .md file and then build - you should get a single .webp image. Renaming to .mdx and changing nothing else, will yield the same screenshot being output twice - once as .webp and once as .png (renamed, but otherwise identical).

Yes, I am aware that I could post-process the build result and remove the .png files from the output, but it's not in the spirit of this project IMO where the tooling is so good and nothing needs silly tweaking.

Important: was unable to figure out how to add an image to StackBlitz, the repro needs a single Screenshot.png in the assets folder.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-bwwxgj-kxfzr1?file=src%2Fcontent%2Fdocs%2Freference%2Fcool.mdx

Participation

  • I am willing to submit a pull request for this issue.
@delucis
Copy link
Member

delucis commented Feb 9, 2024

Thanks for the issue @ddotlic! I also believe this should work in MDX the same way it does in Markdown, not sure why it might not.

As this behaviour is fully controlled by Astro, which Starlight builds on top of, I will transfer this over to that repository to be looked at.

@delucis delucis transferred this issue from withastro/starlight Feb 9, 2024
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Feb 9, 2024
@Princesseuh Princesseuh added - P3: minor bug An edge case that only affects very specific usage (priority) pkg: mdx Issues pertaining to `@astrojs/mdx` integration feat: assets Related to the Assets feature (scope) and removed needs triage Issue needs to be triaged labels Feb 9, 2024
@Princesseuh
Copy link
Member

Princesseuh commented Feb 9, 2024

What I can only assume to be our JSX renderer is trying to access $$slot on the image import, since we keep the image on any access, it causes the original image to stay wrongly.

I think the proper fix would be to investigate, why it's trying to access $$slot in the first place, and if that's a valid thing to do, add an exception to the proxy here: https://github.com/withastro/astro/blob/main/packages/astro/src/assets/utils/proxy.ts

@ddotlic
Copy link
Author

ddotlic commented Mar 2, 2024

@Princesseuh Thank you! Can confirm that this does indeed resolve this issue. Cheers!

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: assets Related to the Assets feature (scope) pkg: mdx Issues pertaining to `@astrojs/mdx` integration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants