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

fix: jsx is undefined by default, not preserve or react #1102

Merged
merged 4 commits into from
Sep 18, 2020

Conversation

agilgur5
Copy link
Contributor

@agilgur5 agilgur5 commented Sep 17, 2020

Description

  • one gets an error when using JSX without configuring jsx:
    error TS17004: Cannot use JSX unless the '--jsx' flag is provided.

  • TSConfig Reference said default was react, which is inaccurate

  • Compiler Options said default was preserve, which is also inaccurate

  • Add a undefined as the default in TSConfig Reference, same as
    allowUnrechableCode

  • Remove the default in Compiler Options, same as all the other options
    without defaults

History:

Reference:

Compiler Options:

Tags

Similar to #1101 , found while investigating discrepancies between the TSConfig Reference and Compiler Options after finding #1092

See historical references above as well

Review Notes

Can remove the undefined default and just have nothing instead in the Reference. Wasn't sure if I should match allowUnreachableCode or if that were an exception since undefined means something else there.

- one gets an error when using JSX without configuring `jsx`:
  `error TS17004: Cannot use JSX unless the '--jsx' flag is provided.`

- TSConfig Reference said default was `react`, which is inaccurate
- Compiler Options said default was `preserve`, which is also inaccurate

- Add a `undefined` as the default in TSConfig Reference, same as
  allowUnrechableCode
- Remove the default in Compiler Options, same as all the other options
  without defaults

----
History:

Reference:
- `preserve` was written as the default in microsoft@eae5cc3#diff-444415c73770eafd0f9db7d708d74131R5
- But then was written as `react` in the body in microsoft@eae5cc3#diff-444415c73770eafd0f9db7d708d74131R5
- It was then switched to `react` in microsoft@450ae96#diff-a912c6af3a16bf4288093c1264955bc6R120
- And then removed in microsoft@76b7d43#diff-a912c6af3a16bf4288093c1264955bc6L120

Compiler Options:
- `preserve` was written as the default in microsoft@66f17ad#diff-bfc78b96c15b0ee906c1ab6176546dceR41
@ghost
Copy link

ghost commented Sep 17, 2020

CLA assistant check
All CLA requirements met.

@orta
Copy link
Contributor

orta commented Sep 18, 2020

I think wiping the string is 👍🏻 - this one also hit the CLA ( maybe it's on the size of the PR?) - good to go if you accept it

@agilgur5
Copy link
Contributor Author

CLA accepted. 🤷 didn't really see a pattern to why it chose this and #1111 to ask, sounds like a potential bug in the CLA checker 😬

@orta orta merged commit b929d9e into microsoft:v2 Sep 18, 2020
@orta
Copy link
Contributor

orta commented Sep 18, 2020

Yeah, I'm not sure I want to look behind the curtain of the wizard of CLA - thanks, this is super useful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants