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

Fix @astrojs/prism edgecase with pnpm #6485

Merged
merged 1 commit into from
Mar 9, 2023
Merged

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Mar 9, 2023

Changes

Fixes #6402

Firstly, I don't like what I'm doing, but it's the best solution at the mean time 😬

This PR makes astro add prismjs/components/index.js to vite.ssr.external in dev time to fix using @astrojs/prism in a pnpm setup.

This is needed because while we do automatic externalizing of transitive deps of Astro libraries, in which case we externalize prismjs by default, prismjs/components/index.js is not covered. In this case, when Vite decides whether to external or noExternal this import, it checks whether it's import-able from the project root. With pnpm, it's not, so it gets noExternal and that doesn't work with CJS files.

Ideally the fix would be in Vite to externalize things base on the real import path, in which case should be @astrojs/prism instead of the project root.

Testing

Tested manually with the issue repo.

Docs

This PR fixes this edgecase so I think we don't need docs for it. The issue in general is a bit hard to explain in docs without digging into Vite internals.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Mar 9, 2023
@changeset-bot
Copy link

changeset-bot bot commented Mar 9, 2023

🦋 Changeset detected

Latest commit: 0438458

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@matthewp matthewp merged commit d637d1e into main Mar 9, 2023
@matthewp matthewp deleted the fix-astro-prism-support branch March 9, 2023 19:44
@astrobot-houston astrobot-houston mentioned this pull request Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@astrojs/prism + pnpm doesn't work together
2 participants