fix(renderer): omitting internal symbol from mdx props

[Xetera]

I've noticed that when using the following code in an mdx file with a content collection, everything works fine if I have a properly compiling page

<MyComponent {...props} />

But if there's an error anywhere in the codebase after a react dependency is being used, let's say

import { useState } from 'react';
export default function MyComponent() {
  useState(0);
  someKindOfRenderBreakingErrorHere;

  return <p>something</p>;
}

we're greeted with an error that makes no sense

Cannot read properties of null (reading 'useState')

But if the error happens earlier in the code like

import { useState } from 'react';
export default function MyComponent() {
  someKindOfRenderBreakingErrorHere;
  useState(0);

  return <p>something</p>;
}

then we get the expected error

someKindOfRenderBreakingErrorHere is not defined

Here's a minimal reproducible example: https://codesandbox.io/p/devbox/magical-hoover-phk2cv?file=%2Fastro.config.mjs%3A9%2C2

After some digging, I found that the problem happens because the symbol

const hasTriedRenderComponentSymbol = Symbol('hasTriedRenderComponent');

gets passed down to the user's component.

The weird error messages even go away if we manage to get rid of that symbol in the mdx outside the compiler before the react component is rendered, like:

{(function (props) {
  const [symbol] = Object.getOwnPropertySymbols(props)
  delete props[symbol]
}(props))}

While trying to unknowningly recreate the above example using an old astro version before v4.5.9, I realized that all react mdx components used to have this issue even without {...props} being spread.

<MyComponent />

After updating astro to latest, I traced this problem getting partially fixed with this commit 627e47d.

I'm guessing it has something to do with Skip.symbol always being passed in, but the new symbol is also always being passed in. I guess I'm not familiar enough with the astro compiler internals to know why that was always erroring and the above commit made it only break when props is spread. Oh well.

Here's the same problem happening with v4.5.8 without any props getting passed to the component in the mdx file in case you're curious https://codesandbox.io/p/devbox/compassionate-hypatia-glvdg5?file=%2Fastro.config.mjs

Changes

I'm not 100% sure what part of the process vnode.type() exactly represents. Omitting the symbol here seems to magically solve the problem, but I can't really explain why a symbol being passed to a component would cause React to be null. Since this is a build error, I can't compare bundle outputs either to see what exactly is being changed either.

Since this function continues to recurse with the value of vnode.props, I had to spread the symbol out of it instead of mutating.

Testing

I added a regression test for the user-facing part of the problem which is that error messages get messed up during build errors. I don't think it would be valuable to check for any specific symbol in the mdx here

Docs

Shouldn't need a documentation change for a bug fix

[changeset-bot]

🦋 Changeset detected

Latest commit: fed1124

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]

!bench render

[ematipico]

!bench render

[github-actions]

PR Benchmark

Render

Page	Avg (ms)	Stdev (ms)	Max (ms)
/astro	110.98	32.46	219.26
/md	3.44	2.95	29.15
/mdx	103.77	21.19	194.76

Main Benchmark

Render

Page	Avg (ms)	Stdev (ms)	Max (ms)
/astro	114.70	31.17	212.31
/md	3.45	2.90	28.97
/mdx	104.36	20.80	197.03

[florian-lefebvre]

@Xetera from the build error logs, looks like the lockfile is broken. Can you resolve it?

[Xetera]

Had to go down a major pnpm version to install properly but I think maybe the changeset detection doesn't like force pushes? I try to keep the commit history clean

[florian-lefebvre]

Had to go down a major pnpm version to install properly

You can use corepack to take care of the pnpm version for you btw

I try to keep the commit history clean

You don't have to if it makes things harder for you, it will be squashed once merging into main

---

Looks like you disabled edits by maintainers, can you either enable it or merge main into your branch?