[code-infra] Add context to plugin-transform-react-constant-elements

[oliviertassinari]

Since we can't git blame in mui-public what's copied from material-ui, I'm adding a comment. I guess from #644 we would fully lose the ability to git diff.

Also side wins: .ts file can't have JSX and the .jsx extension is not allowed.

[mui-bot]

Bundle size report

Bundle	Parsed size	Gzip size
@base-ui-components/react	0B(0.00%)	0B(0.00%)

Details of bundle changes

[Janpot]

and the .jsx extension is not allowed.

It's hard call that a win, JSX code in .js files has been giving me a headache for years 😄

All joking aside, if .jsx extension is not allowed, then we should express that in a single place only, a eslint rule. All other tooling should be built to operate on all valid javascript files, inlcuding .jsx files. That gives us maximum freedom to change our mind some day.

[oliviertassinari]

if .jsx extension is not allowed

@Janpot The thought is that in the past we didn't use it, and in the future, we are migrating everything to TypeScript, so there seems to there should be no reason to have one .jsx file. So it looks like we can simplify our infra by not having to support it.

In practice, the ones I see in https://github.com/search?q=org%3Amui+path%3A*.jsx&type=code&p=1 seems to be about the need to support .js for Vite examples because it goes against Evan's incentive to make React ubiquitous / he's used to the .vue extension files, from the Tweet threads in vitejs/vite#13715 (comment).

I reverted this part; it's too far from the main goal of the PR.

JSX code in .js files has been giving me a headache for years

Would this fix it https://x.com/kentcdodds/status/1362418720623054848 (we never use .ts, only .tsx)? This seems OK.

[Janpot]

So it looks like we can simplify our infra by not having to support it.

I'm arguing that building all of our tooling for any file that can potentially contain javascript is the simplest solution. Otherwise we have this "no .jsx" rule, that has no-one expects as nobody does this in their project, lingering in all of our tooling.

Would this fix it https://x.com/kentcdodds/status/1362418720623054848 (we never use .ts, only .tsx)? This seems OK.

Somewhat yes, at the moment not all of our .ts files can be renamed to .tsx without breaking the parser, but the changes should be minimal. We do use .ts sometimes.

// valid in a .ts file, invalid in a .tsx file:
const x = <T>(t: T) => null;

What he describes is only a small nuisance though for me. I don't think it would be helpful to be absolutist about this. In a React project it makes sense to say "we write .tsx only", but again that should be a single lint rule that enforces that, all other rules should just operate on any typescript file. Why? because not all of our projects are react projects. And some day there may be a valid need to relax this rule a bit for some files in a React project (e.g. next.config.ts).

I believe the biggest tech debt at the moment, after the @mui/monorepo GitHub dependency, is the fact that we write js files with manually edited .d.ts files in core repo. I feel like right now everything we try to modernize and improve in code-infra hits this wall and needs hacks and workarounds at best or gets stalled at worst because of this. These are packages that are half built through tsc and half through our own tooling, it's so messy. On top of that most modern tooling don't expect to see JSX code in ts/js files (for good reason, JSX is parsed differently) and not all of them have the extensibility to work around it.
A full conversion to typescript would fix it, and so much more.

[oliviertassinari]

Otherwise we have this "no .jsx" rule, that has no-one expects as nobody does this in their project, lingering in all of our tooling.

@Janpot Right, to work, it would need an eslint rule that lints all files that can contain JavaScript and asks to use the only valid file extensions.

I'm arguing that building all of our tooling for any file that can potentially contain javascript is the simplest solution.

There are only really a few problems that I see collapsing down the number of extensions to a single .tsx one:

1. Remove the need to rename files.
2. When searching files and looking for JavaScript, be certain that you can apply a suffix to filter out the rest.
3. When looking at extension rules, not having this long list: mjs, mjsx, cjs, cjsx, mts, mtsx, cts, ctsx.

The vibe being: "Why is one not more than enough, can't we delete this complexity"

I believe the biggest tech debt at the moment

Right, it's not the biggest priority to fix this, agree. I was thinking more of setting an end goal, so our journey is guided by it.

we write js files with manually edited .d.ts files in core repo.

This one feels unclear: to what I recall, we used to let TypeScript generate the .d.ts files; the generation wasn't perfect, so we moved to manual .d.ts files alongside source (today .tsx).

[Janpot]

This one feels unclear: to what I recall, we used to let TypeScript generate the .d.ts files; the generation wasn't perfect, so we moved to manual .d.ts files alongside source (today .tsx).

Yes, there isn't enough type information in js and the jsdoc types on their own aren't powerful enough to emulate all ts features. That's why there was opted for writing .d.ts files manually. Instead of running tsc to generate output types, the .d.ts files are simply copied. This works somewhat, but is problematic because it doesn't enforce correctness, there are lots of mismatches between .d.ts and .js file because of this, it doesn't take a lot of time strolling through the @mui/system types to find examples of this.
Except that's not the whole story. Next up, we wanted to write some of the new files in typescript, so we did add a tsc pass for the typescript-only files, and keep the copying of the .d.ts files and hope evrything lines up in the end. Now we have mixed compilation, some of the files run through tsc, some run through copying.
I believe the initial idea of manually writing .d.ts files was wrong, we should have stayed with jsdoc types, and instead import complex types from .ts files.

So instead of

// foo.d.ts
export function foo(a: string): number;

// foo.js
export function (a) {
  return Number(a)
}

I believe a better way would have been to

// types.ts
export type ComplexFunction = (a: string): number;

// foo.js
/** @type {import('./types').ComplexFunction} */
export function (a) {
  return Number(a)
}

that way we could have kept the whole compilation in tsc, generate declarations from our js files. Then we could have kept type checking and declaration generation much closer in sync.

[oliviertassinari]

@Janpot Right, I think I got the history wrong:

1. We had no types, those were managed as @types/material-ui, https://www.npmjs.com/package/@types/material-ui by the community.
2. It was messy: always out of sync. So we moved them in the repository [Typings] TypeScript typings for 1.x branch material-ui#7522. [TypeScript] Refactor typings to modules material-ui#7874
3. But we still had to sync types and docs, so we moved to generate prop-types from types. [docs] Move more prop docs into IntelliSense material-ui#20342
4. Then, we moved to .tsx sources, from which we decided to use explicit type definitions to control the .d.ts output.

So it feels like years of slow incremental progress toward the correct approach, it kind of worked, so we never prioritized to fully clean it up 😁

I believe a better way would have been to

Right, this sounds better. But to also mind that with the .d.ts > propTypes migration, eslint proptypes validation gave us some level of sync guarantee. It's probably how we manage to get away with the problem.

A full conversion to typescript would fix it, and so much more.

Right mui/material-ui#15984.