Support TypeScript 3.7 (and drop Node 6 support for direct inst… #6657

Cryrivers · 2019-10-15T04:27:55Z

This PR addresses TypeScript 3.7 supports, which includes:

Optional Chaining (Fixes [TypeScript 3.7] add support for optional chaining #6595)
Null Coalescing (Fixes [TypeScript 3.7] add support for nullish coalescing operator #6609)
asserts keyword (Fixes [TypeScript 3.7] add support for asserts keyword #6574)
declare keyword on class fields (Fixes [TypeScript 3.7] declare modifier on class fields #6661)

Fixes #6569

I’ve added tests to confirm my change works.
(If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
(If the change is user-facing) I’ve added my changes to the CHANGELOG.unreleased.md file following the template.
I’ve read the contributing guidelines.

✨Try the playground for this PR✨

lydell · 2019-10-15T05:43:03Z

Looks like your snapshots aren’t up-to-date?

Cryrivers · 2019-10-15T05:58:10Z

@lydell Yeah, it's still WIP.

Cryrivers · 2019-10-15T06:18:32Z

@suchipi fyi

Cryrivers · 2019-10-15T06:18:48Z

@lydell updated

Cryrivers · 2019-10-15T06:39:31Z

btw @typescript-eslint/typescript-estree has chokidar as a dependency for watch feature, which doesn't make rollup happy. could i get any advice for that?

lydell · 2019-10-15T06:50:28Z

I think it should be possible to stub chokidar out. Maybe @fisker can help?

fisker · 2019-10-15T07:35:04Z

@lydell Even if we can make this work, still new version @typescript-eslint/typescript-estree can't update due to node version problem

like I said in #6601

Notice: @typescript-eslint/typescript-estree@2.3.2 requires node >=8 , so this PR might can't merge until prettier@2

lydell · 2019-10-15T13:23:57Z

Ah, thanks, I forgot about that.

radiosilence · 2019-10-16T09:48:40Z

@lydell Even if we can make this work, still new version @typescript-eslint/typescript-estree can't update due to node version problem

like I said in #6601

Notice: @typescript-eslint/typescript-estree@2.3.2 requires node >=8 , so this PR might can't merge until prettier@2

How long might that be? Surely node 6 counts as legacy now. Node 8 was released 2 years ago. I'm not sure node 6 is even in its LTS maintenance window any more.

SimenB · 2019-10-16T10:34:57Z

I'm not sure node 6 is even in its LTS maintenance window any more.

6 has been EOL since April. (12 will be LTS Monday next week)

https://github.com/nodejs/Release/blob/master/README.md

Cryrivers · 2019-10-17T13:22:54Z

@duailibe @ikatyang could i know what you think about dropping the support for Node 6?

fisker · 2019-10-17T15:37:51Z

#3919

@Cryrivers

alexander-akait · 2019-10-17T15:54:20Z

#3919 (comment)

I think it is time to drop node 6 for direct-from-GitHub installs

radiosilence · 2019-10-17T16:44:54Z

I'm not sure node 6 is even in its LTS maintenance window any more.

6 has been EOL since April. (12 will be LTS Monday next week)

https://github.com/nodejs/Release/blob/master/README.md

Can we drop support for it then?

alexander-akait · 2019-10-17T16:59:33Z

/cc @prettier/core I think it is time to use babel on our source code to avoid breaking change also we drop node 6 for direct-from-GitHub installs, what do you think about it?

lydell · 2019-10-17T20:18:12Z

drop node 6 for direct-from-GitHub installs

Sounds good to me!

Cryrivers · 2019-10-20T02:47:14Z

@fisker could you help advice how to shim chokidar?

fisker · 2019-10-20T09:06:12Z

@Cryrivers

I already merged your code into #6680

About how to make it work, you can read my PR there is build script in another repo.

Cryrivers · 2019-10-20T15:54:03Z

@fisker great, i'll close my PR when yours has been merged.

Cryrivers · 2019-10-22T02:16:13Z

FYI, typescript-estree removed the dependency on chokidar in 2.5.0

Cryrivers · 2019-10-22T02:49:13Z

Now you can try in the playground for this PR!

alexander-akait · 2019-10-22T10:47:14Z

/cc @Cryrivers Thanks for great work

/cc @fisker and @Cryrivers Can you work together (comments/reviews/etc) in one PR? Because we have two PRs #6657 and #6680, it is create unnecessary noise for contributors and developers who wait this

fisker · 2019-10-22T11:41:26Z

actually Its your call, drop node 6, merge this.

support node 6, merge #6680.

they are the same

alexander-akait · 2019-10-22T13:03:36Z

@fisker

actually Its your call, drop node 6, merge this.

It is just proposal, we need consider this

/cc @prettier/core Which solution will we choose?

sosukesuzuki

Formatting some following cases incorrectly removes the parentheses and results in a syntax error when reformatting.

let f = () => ({}?.());
a = () => ({}?.() && a);
a = () => ({}?.()() && a);
(a) => ({}?.()?.b && 0);
(x) => ({}?.()());
(x) => ({}?.().b);
({}?.a().b());
({ a: 1 }?.entries());

We should add OptionalCallExpression to this line to fix this.(And also need to add tests)

However, these occur even in Babel's OptionalChaining format and have not been pointed out so far, so I think we can ignore it in this PR.

thorn0 · 2019-10-26T23:04:02Z

@sosukesuzuki Optional chaining is going to become much more popular after TS 3.7 gets released, so better to catch all these parens issues in this PR and be ready.

sosukesuzuki

Thank you, @Cryrivers and @thorn0, LGTM!

ikatyang · 2019-10-27T08:51:34Z

@Cryrivers

@ikatyang could i know what you think about dropping the support for Node 6?

Sorry for the late response. FWIW, I'm in favor of dropping legacy Node, but we've promised to our user that we won't make breaking changes in minor/patch release (according to semver) so we shouldn't introduce such change in minor release IMO, though I'm still not quite sure if this rule should be applied here. One possible backward-compatible workaround would be to introduce some postinstall scripts to hide those breaking changes until 2.0.

lydell · 2019-10-27T09:05:30Z

@ikatyang Just in case you weren’t aware, we only drop Node 6 support for direct installation from GitHub. The version released on npm will still work – even in Node.js 4. See also: typescript-eslint/typescript-eslint#1108 (comment)

Not sure if that changes your opinion, but just in case 🤷‍♂

ikatyang · 2019-10-27T09:13:38Z

Yeah, I'm aware. I meant that we've explicitly specified node>=6 in package.json, which looks like a promise to me, I'm not sure how many people use it this way though.

thorn0 · 2019-10-27T10:39:16Z

@ikatyang IMHO the contents of package.json on GitHub can't be a promise unless such a promise is documented somehow, otherwise we could call any piece of code a promise.

ikatyang · 2019-10-27T10:48:51Z

@thorn0 Yes, but we did mention that we support installation from GitHub in https://prettier.io/versions.html, that's why I'm still not sure if it is considered a promise. Anyway, this is just a suggestion, not a requirement.

thorn0 · 2019-10-27T11:07:33Z

@ikatyang It's called "next" there though, not 1.x.x.

alexander-akait

Great job 👍

soroushm · 2019-10-28T11:54:19Z

So we are waiting to release :)

kirananto · 2019-10-28T13:28:22Z

Awesome.. waiting for the release.. 💯

alexander-akait · 2019-10-28T13:32:58Z

@ikatyang I agree with @thorn0 , next also can be unstable, for example for 2.0 we also will be have next, but with breaking changes

deser · 2019-10-29T08:36:20Z

Guys, when it's planned to deliver this to npm?

damiangreen · 2019-10-29T09:26:54Z

Is anyone getting Error: Cannot find module 'source-map-support' issues with this?

thorn0 · 2019-10-29T10:35:51Z

@damiangreen What exactly are you doing to get it?

damiangreen · 2019-10-29T12:22:15Z

@thorn0 It appears to happen on our Bamboo build server.

thorn0 · 2019-10-29T12:43:51Z

@damiangreen What exactly is that build server doing when this happens? Do you have any stack traces or something?

damiangreen · 2019-10-29T13:06:43Z

@thorn0 Our devops guys just pulled this down from AWS Console.

I just switched back from "prettier": "https://github.com/Cryrivers/prettier.git",to using "prettier": "^1.18.2", and the error went away.

Note I have the latest "source-map-support": "^0.5.15", in my dependencies

09:01:51
internal/modules/cjs/loader.js:638

09:01:51
throw err;

09:01:51
^

09:01:51
Error: Cannot find module 'source-map-support'

09:01:51
at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)

09:01:51
at Function.Module._load (internal/modules/cjs/loader.js:562:25)

09:01:51
at Module.require (internal/modules/cjs/loader.js:692:17)

09:01:51
at require (internal/modules/cjs/helpers.js:25:18)

09:01:51
at Object.<anonymous> (/usr/src/app/server.js:1:1)

09:01:51
at Module._compile (internal/modules/cjs/loader.js:778:30)

09:01:51
at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)

09:01:51
at Module.load (internal/modules/cjs/loader.js:653:32)

09:01:51
at tryModuleLoad (internal/modules/cjs/loader.js:593:12)

09:01:51
at Function.Module._load (internal/modules/cjs/loader.js:585:3)

thorn0 · 2019-10-29T13:21:09Z

@damiangreen You still haven't written what this process is doing. How does it use Prettier? Is this stack trace full?

BTW, let's stop spamming everyone who follows this PR. Feel free to open an issue with a proper error report.

Cryrivers marked this pull request as ready for review October 15, 2019 06:18

Cryrivers changed the title ~~WIP: Support TypeScript 3.7~~ Support TypeScript 3.7 Oct 15, 2019

Cryrivers mentioned this pull request Oct 15, 2019

[WIP]Support typescript 3.7 #6600

Closed

4 tasks

timroes mentioned this pull request Oct 17, 2019

Upgrade to TypeScript 3.7.2 elastic/kibana#47188

Merged

15 tasks

fisker mentioned this pull request Oct 17, 2019

Support TypeScript 3.7 (using a fork of typescript-estree) #6680

Closed

4 tasks

alexander-akait added this to the 1.19 milestone Oct 22, 2019

thorn0 requested a review from alexander-akait October 26, 2019 16:43

sosukesuzuki reviewed Oct 26, 2019

View reviewed changes

fix another parens issue, disable some tests for flow

57d740c

thorn0 added 2 commits October 27, 2019 02:21

workaround for flow bug

f11756a

fix issues with comments

42e01ac

sosukesuzuki approved these changes Oct 27, 2019

View reviewed changes

lydell approved these changes Oct 27, 2019

View reviewed changes

bradzacher mentioned this pull request Oct 27, 2019

Optional chaining syntax is still not supported typescript-eslint/typescript-eslint#1144

Closed

alexander-akait approved these changes Oct 28, 2019

View reviewed changes

lydell changed the title ~~Support TypeScript 3.7 (and drop Node 6 support for direct installation from GitHub)~~ Support TypeScript 3.7 (and drop Node 6 support for direct inst… Oct 28, 2019

lydell merged commit 2cfd8fb into prettier:master Oct 28, 2019

bradzacher mentioned this pull request Oct 28, 2019

Parsing 3.7 with prettier integration typescript-eslint/typescript-eslint#1151

Closed

prettier locked as resolved and limited conversation to collaborators Oct 29, 2019

Support TypeScript 3.7 (and drop Node 6 support for direct inst… #6657

Support TypeScript 3.7 (and drop Node 6 support for direct inst… #6657

Conversation

Cryrivers commented Oct 15, 2019 • edited by thorn0

lydell commented Oct 15, 2019

Cryrivers commented Oct 15, 2019

Cryrivers commented Oct 15, 2019

Cryrivers commented Oct 15, 2019

Cryrivers commented Oct 15, 2019 • edited

lydell commented Oct 15, 2019

fisker commented Oct 15, 2019

lydell commented Oct 15, 2019

radiosilence commented Oct 16, 2019 • edited

SimenB commented Oct 16, 2019 • edited

Cryrivers commented Oct 17, 2019

fisker commented Oct 17, 2019

alexander-akait commented Oct 17, 2019 • edited

radiosilence commented Oct 17, 2019

alexander-akait commented Oct 17, 2019

lydell commented Oct 17, 2019

Cryrivers commented Oct 20, 2019 • edited

fisker commented Oct 20, 2019 • edited

Cryrivers commented Oct 20, 2019

Cryrivers commented Oct 22, 2019

Cryrivers commented Oct 22, 2019

alexander-akait commented Oct 22, 2019

fisker commented Oct 22, 2019

alexander-akait commented Oct 22, 2019

sosukesuzuki left a comment • edited

Choose a reason for hiding this comment

thorn0 commented Oct 26, 2019 • edited

sosukesuzuki left a comment

Choose a reason for hiding this comment

ikatyang commented Oct 27, 2019

lydell commented Oct 27, 2019

ikatyang commented Oct 27, 2019

thorn0 commented Oct 27, 2019

ikatyang commented Oct 27, 2019

thorn0 commented Oct 27, 2019

alexander-akait left a comment

Choose a reason for hiding this comment

soroushm commented Oct 28, 2019

kirananto commented Oct 28, 2019

alexander-akait commented Oct 28, 2019

deser commented Oct 29, 2019

damiangreen commented Oct 29, 2019

thorn0 commented Oct 29, 2019

damiangreen commented Oct 29, 2019

thorn0 commented Oct 29, 2019

damiangreen commented Oct 29, 2019 • edited

thorn0 commented Oct 29, 2019 • edited

Cryrivers commented Oct 15, 2019 •

edited by thorn0

Cryrivers commented Oct 15, 2019 •

edited

radiosilence commented Oct 16, 2019 •

edited

SimenB commented Oct 16, 2019 •

edited

alexander-akait commented Oct 17, 2019 •

edited

Cryrivers commented Oct 20, 2019 •

edited

fisker commented Oct 20, 2019 •

edited

sosukesuzuki left a comment •

edited

thorn0 commented Oct 26, 2019 •

edited

damiangreen commented Oct 29, 2019 •

edited

thorn0 commented Oct 29, 2019 •

edited