fix: fix error with <Prism /> component in Cloudflare Workers

[rururux]

fixes: #15722

Changes

• Ported loadLanguages from prismjs, rewrote them as ESM, and configured them to be loaded in Cloudflare Workers environments.
• Accordingly, changed the runHighlighterWithAstro function to an async function.
• Updated files that depended on runHighlighterWithAstro to also handle it as an async function.

Fixes a CommonJS-related error that occurred when using the Cloudflare Workers integration with the <Prism /> component in the latest beta of Astro.
The root cause was that the loadLanguages function exported from prismjs used require() and require.resolve() internally.
To work around this, i ported loadLanguages and its dependencies from the prismjs repository, rewrote them as ESM, and configured them to be loaded only in Cloudflare Workers environments.
Initially, i considered simply adding 'prismjs/components/index.js' to optimizeDeps.include in @astrojs/cloudflare, but this approach was abandoned because while require() was replaced, require.resolve() remained as-is and still caused an error.

---

In the original Prism code, language data files were loaded dynamically like this:

const pathToLanguage = './prism-' + lang;
// ...
require(pathToLanguage);

Since this couldn't be directly reproduced in ESM (the import path needs to be statically known at build time), I took the following approach instead:

• First, I made the behavior switch between the node environment and the workerd environment. In the node environment, things work the same way as before.
• In the workerd environment, a special object bundledLanguages is imported from 'virtual:astro-cloudflare:prism', and each required language file is loaded from there.
• 'virtual:astro-cloudflare:prism' is dynamically generated by a Vite plugin called cfPrismPlugin, which is added in this pull request.
• bundledLanguages looks something like this. This ensures that the import paths are statically known at the time the language data files are loaded:

const bundledLanguages = {
  "javascript": () => import('prismjs/components/prism-javascript.js'),
  "typescript": () => import('prismjs/components/prism-typescript.js'),
  // ...
}

// usage
const lang = 'javascript'
// this will result in `import('prismjs/components/prism-javascript.js')` being called
await bundledLanguages[lang]()

• While this makes things work in the workerd environment, it also means a large number of dynamic import statements are generated, which caused lengthy logs to appear when they were loaded. This PR also addresses that logging issue.

Reference

loadLanguages: https://github.com/PrismJS/prism/blob/76dde18a575831c91491895193f56081ac08b0c5/components/index.js

Testing

Added a test to verify that the component works without errors in a workerd environment.

Docs

N/A, bug fix

[changeset-bot]

🦋 Changeset detected

Latest commit: 1e1d716

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

[rururux]

🤔

[ematipico]

@rururux if the code that we're attempting to load is CJS, we should use vite.resolve.optimiseDeps and add the file to it, that should work

[rururux]

@ematipico

Thank you for the suggestion! I tried the approach you proposed, and while it does improve stability, prismjs is only depended on by @astrojs/prism within Astro's packages, so for projects using the Cloudflare integration without the <Prism /> component (projects that don't depend on @astrojs/prism), the following log now appears:

20:07:54 [WARN] [vite] Failed to resolve dependency: @astrojs/prism > prismjs, present in ssr 'optimizeDeps.include'
20:07:54 [WARN] [vite] Failed to resolve dependency: @astrojs/prism > prismjs/components.js, present in ssr 'optimizeDeps.include'
20:07:54 [WARN] [vite] Failed to resolve dependency: @astrojs/prism > prismjs/dependencies.js, present in ssr 'optimizeDeps.include'

[rururux]

Also, I just noticed there's actually an issue with the implementation in this PR, getLoader().load() was missing an argument that is required when the function passed to load() returns a Promise, which means it wasn't being awaited as expected.
I'll fix this right away.

edit: done

[rururux]

I added a simple detection logic to the Cloudflare Workers integration's Vite plugin that checks whether @astrojs/prism is installed in the current project. By conditionally including prismjs-related files in optimizeDeps.includes based on this result, the log issue should be resolved (verified locally).

One limitation to note: since the check works by reading the package.json in the root folder, it won't detect cases where @astrojs/prism is an indirect dependency of something installed at the root.
I also tried using require.resolve('@astrojs/prism') for the check, but since the test fixtures specify @astrojs/prism as workspace:*, the resolved path doesn't come out as expected, so I went with the package.json approach for now.

If you'd prefer, I can update the Cloudflare integration's test fixtures to install @astrojs/prism from npm instead of workspace:*, and adjust the detection logic accordingly.

[ematipico]

I need to wrap my head with the new code. There are weird things

[rururux]

I've switched to an approach that uses a virtual module to load Prism language files, but only in the Cloudflare Workers environment.
This eliminates the need to reference import.meta.glob() or /node_modules/ directly.

[matthewp]

how does this work? What causes this to be loaded?

[rururux]

This is due to this feature.
In PR #15565 as well, a similar approach was taken to address an issue that only occurs in the workerd environment.

https://developers.cloudflare.com/workers/wrangler/bundling/#conditional-exports

Wrangler respects the conditional exports field ↗ in package.json. This allows developers to implement isomorphic libraries that have different implementations depending on the JavaScript runtime they are running in. When bundling, Wrangler will try to load the workerd key ↗. Refer to the Wrangler repository for an example isomorphic package ↗.

[matthewp]

Creating a preview release to test with.

[pkg-pr-new]

astro

npm i https://pkg.pr.new/astro@15723

@astrojs/prism

npm i https://pkg.pr.new/@astrojs/prism@15723

@astrojs/cloudflare

npm i https://pkg.pr.new/@astrojs/cloudflare@15723

@astrojs/markdoc

npm i https://pkg.pr.new/@astrojs/markdoc@15723

@astrojs/markdown-remark

npm i https://pkg.pr.new/@astrojs/markdown-remark@15723

commit: eabe7ed

[rururux]

I tested this with the preview build and it mostly works fine.
https://github.com/rururux/astro-cf-prism-preview
https://dc96c388-prism-test.rururux.workers.dev/

But I occasionally encountered the error [ERROR] [vite] ReferenceError: Prism is not defined.

I believe the cause is that after globalThis.Prism is deleted in this cleanup section, something tries to read the value. However, removing the cleanup means the Prism object ends up persisting on globalThis indefinitely.

I also tried verifying the behavior with the node adapter, I read globalThis.Prism inside an Astro component that uses the <Prism /> component, and it was not undefined, confirming that the Prism object is being set on globalThis.

I have two proposals in mind:

1. Match the behavior of the node adapter and leave the Prism object set on globalThis.Prism

   • Pros: Consistent behavior with the node adapter regarding whether a value is set on globalThis.Prism
   • Cons: globalThis remains polluted

2. Track the number of invocations or similar, and ensure the Prism object is properly cleaned up from globalThis.Prism

   • Pros: Avoids polluting globalThis
   • Cons: Inconsistent behavior with the node adapter regarding whether a value is set on globalThis.Prism

For now, I'd like to go with option 2, prioritizing keeping globalThis clean.
If you think option 1 would be better, please let me know and I'll update accordingly.

[rururux]

Thanks everyone for the reviews and approvals!
I'll go ahead and merge this shortly unless there are any concerns. 😄