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

Add option to output .js files while preserving jsx #13636

Merged
merged 7 commits into from
Jan 24, 2017

Conversation

ds300
Copy link
Contributor

@ds300 ds300 commented Jan 23, 2017

This PR adds the ability to preserve jsx in source code, but also to output .js files rather than .jsx files. This is useful for react-native which does not support .jsx files.

This is a replacement PR for #9864 which seems to have stalled. It also solves #11158.

David Sheldrick added 2 commits January 23, 2017 18:17
This commit adds the ability to preserve jsx in source code, but
also to output .js files rather than .jsx files. This is useful
for react-native which does not support .jsx files.
@msftclas
Copy link

Hi @ds300, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@RyanCavanaugh
Copy link
Member

Thanks! We were thinking the option name should just be react-native for succinctness/clarity.

@ds300
Copy link
Contributor Author

ds300 commented Jan 23, 2017

A much better name :) I'll make the change later tonight.

@sheetalkamat
Copy link
Member

You also need to enable notification for es5 transformation for new option. Refer: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/transformers/es5.ts#L17

@sheetalkamat
Copy link
Member

Also please add tests in tests\cases\compiler for easy baseline verification of this scenario. Thank you

@ds300
Copy link
Contributor Author

ds300 commented Jan 23, 2017

@sheetalkamat I think I made the appropriate change in es5.ts, but am not sure whether I'm doing the test cases correctly. There doesn't seem to be any notifications in the baseline outputs anyway. Do you mind taking a look?

Copy link
Member

@sheetalkamat sheetalkamat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave a comment

@@ -0,0 +1,22 @@
/// <reference path="fourslash.ts" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be a fourslash test (they are much slower than compiler baselines). See other tests in tests/cases/compiler for how to write a multi-file baseline test (search "@filename")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other compiler testcases I made after this also test the PR's behaviour, so I suppose I can just delete this fourslash one.

@@ -1798,9 +1798,10 @@ declare namespace ts.server.protocol {
namespace JsxEmit {
type None = "None";
type Preserve = "Preserve";
type ReactNative = "ReactNative";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you revert the changes to this file, the file is generated as part of the build. you have edited the correct file under src/server/protocol.ts

@mhegazy mhegazy merged commit 4888e4f into microsoft:master Jan 24, 2017
@ds300 ds300 deleted the preserve-jsx-but-output-.js-files branch January 24, 2017 07:30
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants