Valerioageno / tuono

Superfast fullstack react framework
https://tuono.dev
MIT License
98 stars 5 forks source link

React `JSX` global namespace is deprecated #84

Closed marcalexiei closed 18 hours ago

marcalexiei commented 2 days ago

Greetings, I just tried to setup my first tuono project using

tuono new [NAME]

Everything runs correctly but I want to report that the documentation (e.g.: Components page) and template files (e.g.: src/routes/index.tsx) are using JSX global namespace from @types/react which is now deprecated:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/80e20dcf70052f5bacb051f641ea507e705aba5b/types/react/index.d.ts#L4302-L4306

declare global {
    /**
     * @deprecated Use `React.JSX` instead of the global `JSX` namespace.
     */
    namespace JSX {

If you wish I can setup a PR that import React in all the relevant template and documentation files.

Valerioageno commented 2 days ago

Good call! Did you have problem with errors on your code editor for that? I'm not a huge fan of having a React import on any file. Maybe there is the change of using React.JSX to enable that on all the files WDYT?

marcalexiei commented 2 days ago

Did you have problem with errors on your code editor for that?

Yes, curiously on VSCode the text is not strikethrough, but on IntelliJ IDEA it is.


I'm not a huge fan of having a React import on any file.

I personally prefer to have all imports explicated. But I completely understand that is something that might vary from person to person.


Maybe there is the change of using React.JSX to enable that on all the files WDYT?

The only method I could think of is augmenting the global scope using a .d.ts file. Something like

declare global {
  // export all React types
}

IMHO this could do the trick, but having it in templates and documentation might be error prone since some user might not have React types available "globally".

Valerioageno commented 2 days ago

Right, How do the other framework solves this issue? I know that Next.js for instance allow to not import React on every file.

marcalexiei commented 2 days ago

Looking at some next.js examples (I haven't used next.js) I see that in some scenario they import React in each file:

From what I see I think that the React import (used as value) is optional. On the other hand react types are always imported explicitly.

Valerioageno commented 2 days ago

How about removing the deprecated declaration as you mentioned, but also update the "jsx": "react-jsx" property to "jsx": "preserve"? As far as I understood, this is a problem just with Typescript. Fixing the way it gets compiled might do the trick.

Personally, I strongly like avoiding the React declaration on top of every file.

Do you mind trying it out? Right now, I'm focusing on updating the server side API so I don't have room in the short term to update it.

marcalexiei commented 2 days ago

How about removing the deprecated declaration as you mentioned

Personally, I strongly like avoiding the React declaration on top of every file.

I think that using typed name imports could be a compromise:

import type { JSX } from 'react';

Explicitly specifying return types of the functions helps typescript helps improve performance (which can be a real pain in huge projects). Here there are few references:

Additionally these type annotations are required when using isolatedDeclarations. Right now it might seem useless but it might speed up build process a lot, if splitting a library in many packages depending on each other.

import type will not affect runtime values and removed during compilation. A react import will still be present on the top of each tsx files but IMHO the advantages are worth it.


update the "jsx": "react-jsx" property to "jsx": "preserve"? As far as I understood, this is a problem just with Typescript. Fixing the way it gets compiled might do the trick.

I haven't dived deeply in tuono build process yet, so I can't be sure: From a quick test it looks like that build output is the same when changing tsconfig jsx values. Probably the build is executed by vite and tsc is not involved at all. Anyway I don't see how this should affect react types, I'm sorry 😅. Could you please elaborate?


Do you mind trying it out? Right now, I'm focusing on updating the server side API so I don't have room in the short term to update it.

Sure, as I stated in the description I'm more than happy to help 😉.

Which solution do you prefer I try?

  1. remove deprecated type declarations (as I stated in one of the above sections I don't think that changing jsx value affects the final build)
  2. Use named type imports to replace deprecated values JSX values
Valerioageno commented 2 days ago

Agree. Thanks for the dive! I'd go with solution 2. Let's use named imports!