Skip to content

Support Astro v7#454

Merged
delucis merged 11 commits into
expressive-code:mainfrom
ocavue-forks:ocavue/astro7
Jun 25, 2026
Merged

Support Astro v7#454
delucis merged 11 commits into
expressive-code:mainfrom
ocavue-forks:ocavue/astro7

Conversation

@ocavue

@ocavue ocavue commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Closes #451

@netlify

netlify Bot commented Jun 25, 2026

Copy link
Copy Markdown

Deploy Preview for expressive-code ready!

Name Link
🔨 Latest commit 90e2f75
🔍 Latest deploy log https://app.netlify.com/projects/expressive-code/deploys/6a3d0376c47c2b000806f2d4
😎 Deploy Preview https://deploy-preview-454--expressive-code.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment on lines 42 to 44
parentDocument: {
sourceFilePath: ctx.filename,
sourceFilePath: ctx.fileURL?.href,
},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest satteri doesn't have ctx.filename anymore.

https://github.com/bruits/satteri/blob/main/packages/satteri/CHANGELOG.md#070--2026-06-02
The filename option (and the ctx.filename it surfaced to plugins) is now fileURL and only accepts a URL instead of a string.

@ocavue ocavue marked this pull request as ready for review June 25, 2026 07:05
function createSatteriDocumentFile(ctx: HastVisitorContext): RehypeExpressiveCodeDocument {
return {
path: ctx.filename,
path: ctx.fileURL?.href || '',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason to not use fileURLToPath()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤔

@ocavue ocavue Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. For path, based on Claude's analyze, I think leaving it as an empty string is better in createSatteriDocumentFile, 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: '/',
    
    // ... 
  }
}
  1. For sourceFilePath, we can just use an URL format string.

update: implemented in c1c749d

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tricky, while implemented like this in the code component, in the docs, for getBlockLocale, file is documented as:

A VFile instance representing the Markdown document.

And in the Vfile docs, for path, they say it's a path transformed using fileURLToPath() so a real path.

@ocavue ocavue Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want the safest approach, then a fileURLToPath polyfill sounds like a solid solution to me.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want the safest approach, then a fileURLToPath polyfill sounds like a solid solution to me.

Sounds good 👍

It’s a tiny dep, so seems fair enough.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@delucis delucis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @ocavue — thank you! Just one small query on the release approach, but code LGTM!

Comment thread .changeset/sour-cycles-ring.md Outdated
Comment thread .changeset/sour-cycles-ring.md Outdated

@delucis delucis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, let’s get this released! Thanks again @ocavue 💜

@delucis delucis merged commit 9169010 into expressive-code:main Jun 25, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Astro 7 compatibility

3 participants