Fix generated JSX pragmas for new babel

[remcohaszing]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)
• If applicable, I’ve added docs and tests

Description of changes

Each JSX pragma needs to be on its own line for tools like Babel or TypeScript to understand them.

There is a broken pragma in the @mdx-js/preact tests, which was left untouched. Due to a misconfiguration, to fix it, would break type checking.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mdx	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 10, 2024 3:35pm

[wooorm]

They don’t? This works fine?

Do you have proof?

[wooorm]

It looks like Babel tightened this after this was written (in xdm): https://github.com/babel/babel/pull/14012/files vs https://github.com/wooorm/xdm/blame/4713bd50b87dc07cb0e654553e54b599342c3292/lib/plugin/recma-document.js#L68
SWC used to be strict, but loosened it (due to our comments and babel at the time): swc-project/swc#2377

[remcohaszing]

I first encountered this in MDX analyzer. The way it’s used there is unrelated to usage here, but that’s where I noticed. Someone from the Volar team mentioned the pragmas need to be on separate lines, which helped.

I verified this Babel playground only picked up on these pragmas when they are on separate lines. I also verified this in a TypeScript playground (look at the output changes if you add a newline between those pragmas).

I also noticed the Preact tests in the infer JSX types based on the React types, not the Preact types. To fix this requires a bit more work that’s out of scope of this PR, but I would like to fix it in a follow-up PR.

[wooorm]

TypeScript has also changed: babel/babel#13896 (comment). I tried to figure out the PR but the blame is too big and the refactors are too giant so I bailed.

If the Preact problem requires a different fix, how do you know it relates to this? AFAIK TS always has had problems with multiple JSX runtimes at the same time.

I’m fine with using the new format that Babel and TypeScript introduced.
But I’d prefer it to be pretty:

/**
 * @jsxWhatever
 * @jsxStuff asdasd
 */

So like line endings and asterisks and aligned

[remcohaszing]

Although two asterisks at the beginning works, I think one is better. We don’t want this to be accidentally picked up as JSDoc somehow.

Apparently astring dedents block comments. So the comment becomes this:

/*
* @jsxWhatever
* @jsxStuff asdasd
*/

IMO this is slightly prettier:

/*
@jsxWhatever
@jsxStuff asdasd
*/

[remcohaszing]

If the Preact problem requires a different fix, how do you know it relates to this? AFAIK TS always has had problems with multiple JSX runtimes at the same time.

We should leverage TypeScript references to support mono repos and separate projects. There should be only one JSX runtime per project. This is unrelated to the MDX output, which is handled by this PR.

[wooorm]

I dunno, feels weird to have to do that? Why “should” there be one runtime per project? I would think it is pretty reasonable for folks to have different runtimes in one project (with comments in files), and that this is a bug in TS?

[wooorm]

We don’t want this to be accidentally picked up as JSDoc somehow

Why not? Them starting at a line and using @ makes them look a lot like JSDoc. Which I infer to be because they’re intended to look like JSDoc?

IMO this is slightly prettier:

Also fine by me

[remcohaszing]

Why not? Them starting at a line and using @ makes them look a lot like JSDoc. Which I infer to be because they’re intended to look like JSDoc?

I agree with this sentiment. But JSDoc is applied to the identifier it’s above. I don’t want the comment containing JSDoc pragmas to end up as JSDoc for exported functions or _createMdxContent.

---

This might be a nice alternative too:

/* @jsxRuntime automatic */
/* @jsxImportSource react */

[wooorm]

I quite like the separate comments but it’s a bit extra work for the _mdxIsPragmaComment stuff aka this handling: f48d038.

What do you think, go with separate comments?

[remcohaszing]

string strips leading / trailing whitespace 1-liner block comments too, but I still like it slightly better.

[wooorm]

Nitty, but I don‘t really like the reverse stuff.
This works locally (but I can’t suggest it on GH as it overlaps):

    tree.comments.unshift(
      ...pragmas.map(function (d) {
        return /** @type {const} */ ({
          type: 'Block',
          value: d,
          data: {_mdxIsPragmaComment: true}
        })
      })
    )

Only don’t really like the const but TS needs it apparently

[remcohaszing]

I tried this at first, but TS caught me too.

I see the pragmas array is only used here. Instead of creating the pragmas array above, I could also unshift the comment nodes directly on lines 86-100.

[wooorm]

Ahh. I dunno maybe a lot of repetition. Maybe more comments will be added in the future too.
I don‘t mind my const too much? How do you feel about that?

[remcohaszing]

I already applied this using a function to avoid repetition. I like it, but I can revert if you don’t.

[wooorm]

Released in 3.0.1! Thanks!

[wooorm]

This PR broke the website. Looks like esbuild doesn’t pick the manual pretty comments up.

[wooorm]

Both

/* @jsxRuntime automatic */
/* @jsxImportSource react */

And:

/* @jsxRuntime automatic @jsxImportSource react */

Are fine. So the release is not broken.