Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make default JSX mode "react" #8529

Open
donaldpipowitch opened this issue May 9, 2016 · 33 comments
Open

Make default JSX mode "react" #8529

donaldpipowitch opened this issue May 9, 2016 · 33 comments
Labels
Good First Issue Well scoped, documented and has the green light Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@donaldpipowitch
Copy link
Contributor

To enable JSX syntax in TypeScript you need two steps:

  • add "jsx": "react" to "compilerOptions" in tsconfig.json
  • use .tsx extension

This is quite troublesome if you want to reuse certain generators/boilerplates/build scripts for us. It is also against the single source of truth principle and can be confusing. I recently stumbled over a bug where we used the .tsx extension, but hadn't configured the tsconfig.json.

There was a lot of discussion recently about changing the .js extension to .mjs to introduce ES6 modules to Node and many people are against that and prefer to have an additional flag in the package.json.
I'd like to see the same for TypeScript and JSX:

  • the .tsx extension is purely optional
  • only "jsx": "react" in tsconfig.json is mandatory
@mhegazy
Copy link
Contributor

mhegazy commented May 9, 2016

only "jsx": "react" in tsconfig.json is mandatory

it is not mandatory. the default is "preserve".

only "tsx": "react" in tsconfig.json is mandatory

the extension is used to decide how to parse <id>, in a .tsx file it is parsed as a jsx tag, in a .ts file its parsed as a cast expression. this seems clearer why you can use a language feature or not than a compilation option.

@RyanCavanaugh
Copy link
Member

it is not mandatory. the default is "preserve"

I don't think this is true, unless we've changed it very recently

@mhegazy
Copy link
Contributor

mhegazy commented May 9, 2016

Should the default be "React" instead.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels May 9, 2016
@basarat
Copy link
Contributor

basarat commented May 10, 2016

I've been using the new as syntax for assertion exclusively (and recommending it for others as well). Maybe <> style assertions should be switched off. It should result in compile errors for existing code and they can easily migrate to as.

^ Of course I might be wrong and your judgement is better 🌹

@rozzzly
Copy link

rozzzly commented May 11, 2016

@basarat

Maybe <> style assertions could be be optionally be switched off via a tsconfig.json flag.

FIFY

But in all honestly, this is a fantastic solution to this issue. It is opt-in & non-breaking. I for one find the extension .tsx—and its inbred cousins .jsx and .es6.js, etc—to be quite ugly. Of course, yes, I am indeed quite particular, but I'm sure there are plenty of people out there who are just as anal retentive as I am—eg. .jsx extension is pretty much gone in most of the the new react repos that I have been seeing.

@aluanhaddad
Copy link
Contributor

I don't think this is a good idea. The React syntax is non-standard and may not be compatible with future versions of EcmaScript. Making .tsx optional would seem to couple React with the future of TypeScript for very little benefit.

@donaldpipowitch
Copy link
Contributor Author

There wouldn't be a problem if JSX isn't enabled by default and you must enable it in your tsconfig.json right? The point is that I don't want to use JSX by default, but I only want one place to configure it - the config and not the extension.

@aluanhaddad
Copy link
Contributor

@donaldpipowitch I see I misunderstood. As long as you have to turn it on, it doesn't seem like a problem.

Personally, while I rarely use React, I always use the as operator for type assertions because I find it more readable.

@Arnavion
Copy link
Contributor

Arnavion commented May 12, 2016

FYI there is a tslint rule to enforce as over <>.

@aluanhaddad
Copy link
Contributor

@Arnavion Awesome. I'm going to turn it on right now

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Committed The team has roadmapped this issue Good First Issue Well scoped, documented and has the green light and removed In Discussion Not yet reached consensus labels May 16, 2016
@RyanCavanaugh
Copy link
Member

We'll make the default JSX mode react for convenience, but still need the separate file extensions so we know when to use the TSX grammar. Still no plans to deprecate or disfavor the <T>expr syntax.

This should be a super-easy PR if anyone's interested.

@DanielRosenwasser DanielRosenwasser changed the title Make ".tsx" extension optional Make default JSX mode "react" May 16, 2016
@donaldpipowitch
Copy link
Contributor Author

Okay, but can you explain what are the advantages of two syntaxes? Or why using the tsconfig alone for a project is not enough?

@RyanCavanaugh
Copy link
Member

So let's say you have some code like this:

let x = <foo>bar;
/* 10,000 lines of code follow */

Or this:

let x = <foo>bar;
/* 10,000 lines of code follow */
</foo>;

One of these is a file with a JSX tag, and one of these is a file with a type assertion. But we would have to parse 10,000 lines of code to figure that out! This would make interactive editing very slow.

The .tsx extension "turns off" the <T>expr syntax to eliminate the ambiguity. A project setting isn't really enough here because you might have some existing code that uses <T>expr in a .ts file and we don't want to break that code.

@rozzzly might you provide some feedback along with the 👎 ?

@rozzzly
Copy link

rozzzly commented May 17, 2016

@RyanCavanaugh

The .tsx extension "turns off" the <T>expr syntax to eliminate the ambiguity. A project setting isn't really enough here because you might have some existing code that uses expr in a .ts file and we don't want to break that code.

Ah, I see the reasoning here. My apologies, please disregard my aversion to .tsx, I now see the necessity of it. I suppose the same could be achieved with a comment directive, something akin to /** @jsx React.DOM **/ or // @flow... but now I feel as if I'm just grasping at straws here, requiring.tsx is the way to go I suppose :/

@donaldpipowitch
Copy link
Contributor Author

donaldpipowitch commented May 17, 2016

A project setting isn't really enough here because you might have some existing code that uses expr in a .ts file and we don't want to break that code.

If existing projects are the major problem how about adding another property in the tsconfig.json which turns the .tsxextension to be optional and forbids <T>expr? This would be an opt-in.

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

i thought you did not like multiple configurations in tsconfig.json to work with JSX/TSX...

@donaldpipowitch
Copy link
Contributor Author

donaldpipowitch commented May 17, 2016

I want one "place" (like a file) to enable JSX for TypeScript, so I can easily share or generate boilerplate for a new project for example. I often see developers (like myself) starting a new project with files using the .tsx extension, but forgetting about setting the tsconfig.json, too (or the other way around). Additionally I don't want to distinguish .ts and .tsx in my build scripts (like the entry in a Webpack configuration). Having two properties in the tsconfig.json (one for setting a pragma/framework like "react" and one for enabling JSX in .ts files for the whole project) would solve this.

I didn't count two properties in tsconfig as "multiple configurations" in this case, because they live in the same file and exactly where they belong - in the project configuration file. I dislike having the .tsx extension as an additional "configuration" step.

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

you can always have .tsx only files everywhere. so now you need not worry about boilerplate code setup, you will have to give up on <T> type casts.

@donaldpipowitch
Copy link
Contributor Author

Yeah, this works, but only if you have full control over your project. Say you want to share a generic webpack.config.json for other projects and have to set an entry. Currently you need custom logic to determine, if the entry is index.ts or index.tsx (just as an example).

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

if it is a module it should not matter, the compiler will look for both.

@DanielRosenwasser
Copy link
Member

@mhegazy I think that @donaldpipowitch is talking about the "entry" field in webpack's config objects (as you can see here), but I don't feel like that's a sufficiently compelling scenario.

@frogcjn
Copy link

frogcjn commented Jun 9, 2016

I want to use "jsx": "preserve", but use *.js extension, since RN not support JSX extension.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jun 24, 2016

I still like .tsx because it says "This file contains weird syntax that requires additional tools to actually transpile to pure JavaScript!".

@frogcjn
Copy link

frogcjn commented Jun 24, 2016

@aluanhaddad
This is not decided by us, because RN refuse to read *.jsx. Someone PR a commit to read *.jsx, but Facebook refuse to accept it and then revert it.
See facebook/react-native#5233 (comment)

@aluanhaddad
Copy link
Contributor

@frogcjn Sorry, I didn't realize they actually rejected a PR to fix it on their end. That is an unfortunate attitude considering the degree of effort from third party developers of both languages (ex: Microsoft: TypeScript) and tools (ex: JetBrains: WebStorm) to support jsx syntax.

@yiminghe
Copy link

better make extension name configurable, current situation for react-native project:

  1. use 'react', get .js file, but it will transform jsx and hard to debug.
  2. use 'preserve', get .jsx file, but react-native does not support it.

want to use 'preserve' and get .js file.

@mhegazy mhegazy added this to the TypeScript 2.1 milestone Sep 21, 2016
@mhegazy mhegazy removed this from the TypeScript 2.1 milestone Sep 29, 2016
@mhegazy mhegazy added this to the Community milestone Sep 29, 2016
@mhegazy mhegazy removed the Committed The team has roadmapped this issue label Sep 29, 2016
@bolatovumar
Copy link

Should this issue be closed?

As far as I'm aware you always have to specify the value for the "jsx" flag either in tsconfig.json or through the command line. TS never implicitly sets the "jsx" flag to equal to "preserve". In fact, if you try to compile a .tsx file without specifying the "jsx" option you get an error as such.

capture

If you try to compile without specifying the "jsx" option at all you get an error as such:

capture

Am I missing something here?

@donaldpipowitch
Copy link
Contributor Author

donaldpipowitch commented Jun 11, 2017

The topic of this issue was changed. I originally wanted to find a way to drop the .tsx extension and still use TSX syntax. I still would like to get rid of this for easier build configs.

It then was changed to what the default value of the jsx setting in the tsconfig should be or so. Not sure what the state of this is.

@cameron-martin
Copy link

Additionally I don't want to distinguish .ts and .tsx in my build scripts (like the entry in a Webpack configuration).

Do webpack entry points not use the node resolution algorithm, meaning you can leave the extension off?

@donaldpipowitch
Copy link
Contributor Author

Not in the entry afaik.

@cameron-martin
Copy link

Would pursuing that avenue solve your problem? Making the entry use the node resolution algorithm seems like sensible behaviour.

@donaldpipowitch
Copy link
Contributor Author

Not sure if I understand your question, but the webpack use case with entry was just one example. I'd like to overall reduce possible solutions: e.g. I often see devs tediously renaming files from .ts to .tsx and back, because they added/removed React/JSX syntax. Of course I could tell them to always use .tsx, but most tutorials use .ts and it just seems to be something which new developers don't always "get". I'd also like to reduce the cast syntax to just as which seems to be easier to "get" in my experience.

@hasparus
Copy link

hasparus commented Oct 11, 2018

Is this issue still open? I'd love to give it a shot, but I have a question.
Current default is "preserve" -- the build succeeds, preserves JSX and logs an error:

error TS17004: Cannot use JSX unless the '--jsx' flag is provided.
  5     <div
[...]

Do we want to change the default value to "react" but still log an error or change the value and get rid of the error? IMHO allowing to use the default without the error would be a little bit nicer.

TypeScript versions I've tested it with are 3.1.2 and my clone of today's master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Well scoped, documented and has the green light Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests