Fix broken restoration of remark directives.

[tritao]

Description

This fixes the logic in remarkDirectivesRestoration so it only restores remark directives that do not have node data.

If the node has a defined data, this means it was set explicitly and should be allowed to continue to be processed later on by remark-rehype.

[changeset-bot]

🦋 Changeset detected

Latest commit: e8f0ae4

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

This PR includes changesets to release 1 package

Name	Type
@astrojs/starlight	Patch

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

[netlify]

✅ Deploy Preview for astro-starlight ready!

Name	Link
🔨 Latest commit	e8f0ae4
🔍 Latest deploy log	https://app.netlify.com/sites/astro-starlight/deploys/66ea838ffac0a7000845dce0
😎 Deploy Preview	https://deploy-preview-2327--astro-starlight.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 100 (no change from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 92 (no change from production) PWA: - View the detailed breakdown and full score reports

---

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

[HiDeoo]

Thanks for your contribution 🙌

On a pure technical level, the changes make sense to me however I'm having a hard time thinking of a use case/scenario for this usage. Considering we would probably want to add a test case for this (e.g. something in the lines of this existing one but with a remark plugin setting some data and a rehype plugin), could you provide more details on how you're currently hitting this issue?

[tritao]

Sure, my use case is quite simple, transform a directive like :symbol[class] to a specific icon, see https://github.com/tritao/remark-symbol-directives/blob/main/index.ts#L32

Without this change, my transformation is just ignored.

[HiDeoo]

Oh, I see, I was definitely starting to imagine more complicated scenarios. This makes sense, thanks for the clarification 🙌

I assume we could definitely have a test for this case too, I imagine it would looks something like this:

test('does not transform back directive nodes with data', async () => {
	const processor = await createMarkdownProcessor({
		remarkPlugins: [
			...starlightAsides({
				starlightConfig,
				astroConfig: {
					root: new URL(import.meta.url),
					srcDir: new URL('./_src/', import.meta.url),
				},
				useTranslations,
			}),
			// A custom remark plugin updating the node with data that should be consumed by rehype.
			function customRemarkPlugin() {
				return function transformer(tree: Root) {
					visit(tree, (node) => {
						if (node.type !== 'textDirective') return;
						node.data ??= {};
						node.data.hName = 'span';
						node.data.hProperties = { class: `api` };
					});
				};
			},
			remarkDirectivesRestoration,
		],
	});

	const res = await processor.render(`This method is available in the :api[thing] API.`);
	expect(res.code).toMatchInlineSnapshot(
		`"<p>This method is available in the <span class="api">thing</span> API.</p>"`
	);
});

I did not try it, just tweaked the existing test I linked earlier and I imagine this test would fail without your fix and pass after. Would you be up for trying to add such a test? If not, let me know and I can figure it out.

[tritao]

Oh, I see, I was definitely starting to imagine more complicated scenarios. This makes sense, thanks for the clarification 🙌

I assume we could definitely have a test for this case too, I imagine it would looks something like this:

test('does not transform back directive nodes with data', async () => {
	const processor = await createMarkdownProcessor({
		remarkPlugins: [
			...starlightAsides({
				starlightConfig,
				astroConfig: {
					root: new URL(import.meta.url),
					srcDir: new URL('./_src/', import.meta.url),
				},
				useTranslations,
			}),
			// A custom remark plugin updating the node with data that should be consumed by rehype.
			function customRemarkPlugin() {
				return function transformer(tree: Root) {
					visit(tree, (node) => {
						if (node.type !== 'textDirective') return;
						node.data ??= {};
						node.data.hName = 'span';
						node.data.hProperties = { class: `api` };
					});
				};
			},
			remarkDirectivesRestoration,
		],
	});

	const res = await processor.render(`This method is available in the :api[thing] API.`);
	expect(res.code).toMatchInlineSnapshot(
		`"<p>This method is available in the <span class="api">thing</span> API.</p>"`
	);
});

I did not try it, just tweaked the existing test I linked earlier and I imagine this test would fail without your fix and pass after. Would you be up for trying to add such a test? If not, let me know and I can figure it out.

Sure, no problem, I added the test you provided, it worked perfectly. Also confirmed the test fails without my fix.

[delucis]

Thanks for fixing this @tritao and for helping debug and test @HiDeoo. Looks like this should be ready for our next release 🚀