microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.04k stars 12.49k forks source link

Make default JSX mode "react" #8529

Open donaldpipowitch opened 8 years ago

donaldpipowitch commented 8 years ago

To enable JSX syntax in TypeScript you need two steps:

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:

mhegazy commented 8 years ago

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 commented 8 years ago

it is not mandatory. the default is "preserve"

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

mhegazy commented 8 years ago

Should the default be "React" instead.

basarat commented 8 years ago

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 :rose:

rozzzly commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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

aluanhaddad commented 8 years ago

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

RyanCavanaugh commented 8 years ago

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.

donaldpipowitch commented 8 years ago

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 commented 8 years ago

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 :-1: ?

rozzzly commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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

donaldpipowitch commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

DanielRosenwasser commented 8 years ago

@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 commented 8 years ago

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

aluanhaddad commented 8 years ago

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

frogcjn commented 8 years ago

@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 https://github.com/facebook/react-native/pull/5233#discussion_r49682279

aluanhaddad commented 8 years ago

@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 commented 8 years ago

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.

bolatovumar commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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 commented 6 years ago

Not in the entry afaik.

cameron-martin commented 6 years ago

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

donaldpipowitch commented 6 years ago

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 commented 6 years ago

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.