Support Astro v7#454
Conversation
✅ Deploy Preview for expressive-code ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| parentDocument: { | ||
| sourceFilePath: ctx.filename, | ||
| sourceFilePath: ctx.fileURL?.href, | ||
| }, |
There was a problem hiding this comment.
The latest satteri doesn't have ctx.filename anymore.
https://github.com/bruits/satteri/blob/main/packages/satteri/CHANGELOG.md#070--2026-06-02
Thefilenameoption (and thectx.filenameit surfaced to plugins) is nowfileURLand only accepts aURLinstead of a string.
| function createSatteriDocumentFile(ctx: HastVisitorContext): RehypeExpressiveCodeDocument { | ||
| return { | ||
| path: ctx.filename, | ||
| path: ctx.fileURL?.href || '', |
There was a problem hiding this comment.
Any specific reason to not use fileURLToPath()?
There was a problem hiding this comment.
Cloudflare adapter doesn't like fileURLToPath from node:url.
FAIL test/integration.test.ts > Integration into Astro ^4.5.0 with Cloudflare adapter
Error: Command failed with exit code 1: pnpm astro build
Error: 4 [ERROR] [vite] x Build failed in 7.27s
[commonjs--resolver] [plugin vite:resolve] Cannot bundle Node.js built-in "node:url" imported from "../../../dist/index.js". Consider disabling ssr.noExternal or remove the built-in dependency.
Stack trace:
at getRollupError (file:///home/runner/work/expressive-code/expressive-code/node_modules/.pnpm/rollup@4.55.1/node_modules/rollup/dist/es/shared/parseAst.js:401:41)
Full CI log with error message: https://github.com/ocavue-forks/expressive-code/actions/runs/28152026775/job/83371712904#step:7:349
There was a problem hiding this comment.
We could use a fileURLToPath polyfill to support non-Node.js environments, like this one:
https://github.com/sindresorhus/url-extras
What do you think?
There was a problem hiding this comment.
Do we know what uses the path and sourceFilePath fields?
The format of file URL is not the same as a file path so I’m wondering if this will cause issues for anyone downstream.
There was a problem hiding this comment.
I don't know enough about EC but my initial thought was if somewhere it could be an issue to get file:///project/custom%20demo/something instead of /project/custom demo/something 🤔
There was a problem hiding this comment.
- For
path, based on Claude's analyze, I think leaving it as an empty string is better increateSatteriDocumentFile, as shown in existing pattern.
function createSatteriDocumentFile(ctx: HastVisitorContext): RehypeExpressiveCodeDocument {
return {
// Use the recommended `url` field
url: ctx.fileURL,
// Set the fallback `path` and `cwd` as default values.
path: '',
cwd: '/',
// ...
}
}- For
sourceFilePath, we can just use an URL format string.
update: implemented in c1c749d
There was a problem hiding this comment.
It's tricky, while implemented like this in the code component, in the docs, for getBlockLocale, file is documented as:
A
VFileinstance representing the Markdown document.
And in the Vfile docs, for path, they say it's a path transformed using fileURLToPath() so a real path.
There was a problem hiding this comment.
If we want the safest approach, then a fileURLToPath polyfill sounds like a solid solution to me.
There was a problem hiding this comment.
If we want the safest approach, then a
fileURLToPathpolyfill sounds like a solid solution to me.
Sounds good 👍
It’s a tiny dep, so seems fair enough.
Closes #451