typecheck updates & fixes in src/language-js #8759

brodybits · 2020-07-15T04:24:02Z

updated:

add some type support packages to devDependencies
add & update JSDoc type comments
add src/language-js/types/estree.d.ts to facilitate clean parameter types in src/language-js/utils.js
apply some minor code updates to resolve some typecheck issues
~~add a single @ts-ignore comment, to work around a type issue remaining in src/language-js/utils.js~~
remove src/language-js & src/common/parser-create-error.js from exclude list in tsconfig.json
move & refactor typedefs in src/language-js/print/ternary.js, based on the changes I had originally proposed in PR refactor typedefs in src/language-js/print/ternary.js #8745
add some blank lines
minor extra cleanup of src/main/comments.js, as described below

(there used to be many more @ts-ignore comments, which are now resolved)

P.S. I suspect some of the minor code updates can make the code a little more robust. It would be interesting to see if we can think of any test cases to prove this or not.

I’ve added tests to confirm my change works.
I’ve read the contributing guidelines.

✨Try the playground for this PR✨

alexander-akait

There are a lot of TODO, can we remove them and adding types late in other PRs?

brodybits · 2020-07-15T13:10:40Z

I just merged changes from master & replaced TODO with TBD (...) in the @ts-ignore comments. I would also like to rebase to improve the history message. My one issue is that I hope we can try to avoid using @ts-ignore comments to avoid resolving new JSDoc type issues in the future.

brodybits · 2020-07-15T23:14:07Z

I just rebased this proposal with a hopefully cleaner commit history, with some updates:

add some conditions to resolve JSDoc type issues, following suggestion in use strict number JSDoc type in src/common/util.js #8760 (comment)
add a few more blank lines
improve @ts-ignore comments in src/language-js/print/call-arguments.js
improve a comment in src/language-js/print/ternary.js

At this point there are 2 ~~TODO~~ comments for issues that I think should be addressed in the near future:

error type issue in src/common/parser-create-error.js
Prettier options object typedef in src/language-js/print/ternary.js

Other @ts-ignore comments are simple TBD comments - see the updated list in the description.

~~It looks like the build cancelled itself, would appreciate it if someone could restart the build.~~

alexander-akait · 2020-07-16T12:22:27Z

/cc @prettier/core

src/language-js/comments.js

src/language-js/needs-parens.js

brodybits · 2020-07-16T22:13:28Z

src/language-js/printer-estree.js

@@ -672,8 +683,9 @@ function printPathNoParens(path, options, print, args) {
            options.locEnd
          );
          return (
+            nextCharacter &&


Suggested change

nextCharacter &&

nextCharacter !== false &&

I wish we had a good way to test this kind of case.

…2388

…st.d.ts

fisker · 2020-08-12T04:12:45Z

Saw this #8759 (comment)?

brodybits · 2020-08-12T04:33:09Z

Saw this #8759 (comment)?

OK so Prettier basically uses its own "estree" format which can handle estree or estree-like AST structure that comes out of the Babel, Flow, or typescript-estree parser modules.

I am wondering about a couple things, though:

We had to add what look like some non-standard fields to some AST node types to get the function type checks to work properly. And we added these fields to all other exported "estree" AST node types.
The output of the Babel, Flow and typescript-estree parsers are processed by postprocess.js before returned by the parse function and are ready to be piped into the printer function.

I will probably take my rest pretty soon. Unfortunately I have some other priorities that I had put off for a while and may not be able to contribute much more over the next 1-2 weeks or so. I hope we can finish this PR without too much more work now and consider improving the type check in future PRs.

This reverts commit ea68916.

brodybits · 2020-08-13T02:31:20Z

Latest estree.d.ts updates in 5414a62 look good to me, thanks.

…nguage-js-jsdoc-types

ExE-Boss · 2020-08-19T15:27:51Z

src/language-js/types/estree.d.ts

+// MUST REMOVE to avoid errors with TypeScript 4.0.0:
+import * as ts from "typescript";
+declare module "typescript" {
+  type NamedTupleMember = Node;


This will work even with TypeScript 4.x:

Suggested change

type NamedTupleMember = Node;

export interface NamedTupleMember extends Node {}

Thanks @ExE-Boss. I would actually favor keeping this workaround as-is, errors can remind us to remove it with the TypeScript 4 update. Nice idea, though.

We should be able to remove this entire workaround if typescript-eslint/typescript-eslint#2405 is merged and released. I hope this will be possible.

… calls", to avoid a merge conflict from master This reverts commit d307f74.

…nguage-js-jsdoc-types

fisker · 2020-08-21T01:08:25Z

Good job

brodybits · 2020-08-21T01:17:50Z

Teamwork yeah!

This was referenced Jul 15, 2020

refactor typedefs in src/language-js/print/ternary.js #8745

Closed

use strict number JSDoc type in src/common/util.js #8760

Draft

alexander-akait reviewed Jul 15, 2020

View reviewed changes

brodybits marked this pull request as draft July 15, 2020 12:53

brodybits force-pushed the language-js-jsdoc-types branch from 8c024a7 to cf142e4 Compare July 15, 2020 22:39

brodybits marked this pull request as ready for review July 15, 2020 22:41

fisker reviewed Jul 16, 2020

View reviewed changes

src/language-js/comments.js Outdated Show resolved Hide resolved

thorn0 reviewed Jul 16, 2020

View reviewed changes

src/language-js/needs-parens.js Show resolved Hide resolved

brodybits changed the title ~~JSDoc type fixes in src/language-js~~ typecheck updates & fixes in src/language-js Jul 16, 2020

brodybits marked this pull request as draft July 16, 2020 21:23

brodybits commented Jul 16, 2020

View reviewed changes

brodybits added 16 commits July 16, 2020 18:44

add @types/node to devDependencies

d2e5a9e

import assert type in src/language-js/needs-parens.js

4bde973

import assert type in src/language-js/printer-estree.js

1959b8e

typedef Doc type in src/document/index.js

cf8fd21

import & use Doc type in src/language-js/print/module.js

6fde6d1

import & use Doc type in src/language-js/printer-estree.js

aed44b2

define & use 2 JSDoc types in src/language-js/utils.js

8b420bf

add type comments to hasLeadingComment in src/language-js/comments.js

f510935

cleanup: JSDoc items to beginning of src/language-js/print/ternary.js

865f365

@typedef {any} Option in src/language-js/print/ternary.js (TBD ...)

9abf8a1

fix(types): value condition in src/language-js/printer-estree.js

a728b7e

fix(types): remove extra args from internal plusOne & plusTwo calls

d307f74

fix(types): remove extra arg in src/language-js/print/member-chain.js

eefa0de

fix(types): add value conditions in src/language-js/comments.js

a5d5ee7

fix(types): value condition in src/language-js/print/member-chain.js

171343a

fix(types): add a type condition in src/language-js/printer-estree.js

82e9caa

brodybits added 4 commits August 11, 2020 23:57

add d.ts extension to lint:prettier script

d988fcf

FIX & UPDATE WORKAROUND HACK for typescript-eslint/typescript-eslint#…

ea7d224

…2388

add extra & comment fields to all exported AST node types in ast.d.ts

983a099

fixup! add extra & comment fields to all exported AST node types in a…

435b9c3

…st.d.ts

brodybits marked this pull request as ready for review August 12, 2020 04:09

brodybits and others added 2 commits August 12, 2020 00:47

Revert "rename estree.d.ts -> ast.d.ts in src/language-js/types"

cd52842

This reverts commit ea68916.

Add types to Node.extra, remove internal types

5414a62

fisker approved these changes Aug 12, 2020

View reviewed changes

fisker requested review from sosukesuzuki and thorn0 August 18, 2020 08:04

brodybits added 3 commits August 18, 2020 13:01

Merge branch 'master' of https://github.com/prettier/prettier into la…

8a6e970

…nguage-js-jsdoc-types

update @types/node -> 14.6.0 in devDependencies

c6662a6

update @typescript-eslint/types -> 3.9.1 in devDependencies

83bd956

brodybits mentioned this pull request Aug 19, 2020

Add espree parser #9000

Merged

4 tasks

ExE-Boss reviewed Aug 19, 2020

View reviewed changes

brodybits mentioned this pull request Aug 19, 2020

fix(typescript-estree): ts.NamedTupleMember workaround for <TS4.0 typescript-eslint/typescript-eslint#2405

Merged

brodybits added 3 commits August 19, 2020 14:08

Revert "fix(types): remove extra args from internal plusOne & plusTwo…

5346498

… calls", to avoid a merge conflict from master This reverts commit d307f74.

Merge branch 'master' of https://github.com/prettier/prettier into la…

1979c45

…nguage-js-jsdoc-types

Merge branch 'master' into language-js-jsdoc-types

29b43a7

fisker merged commit cdc250c into prettier:master Aug 21, 2020

brodybits deleted the language-js-jsdoc-types branch August 21, 2020 01:13

brodybits mentioned this pull request Aug 21, 2020

Upgrade to TypeScript 4.0 #8982

Merged

4 tasks

fisker mentioned this pull request Oct 29, 2020

Use -1 instead of false when index out of bounds #9533

Draft

4 tasks

fisker mentioned this pull request May 31, 2021

Remove @types/node package #10994

Merged

4 tasks

github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

typecheck updates & fixes in src/language-js #8759

typecheck updates & fixes in src/language-js #8759

brodybits commented Jul 15, 2020 •

edited

alexander-akait left a comment

brodybits commented Jul 15, 2020

brodybits commented Jul 15, 2020 •

edited

alexander-akait commented Jul 16, 2020

brodybits Jul 16, 2020

fisker commented Aug 12, 2020

brodybits commented Aug 12, 2020

brodybits commented Aug 13, 2020

ExE-Boss Aug 19, 2020

brodybits Aug 19, 2020

brodybits Aug 19, 2020

fisker commented Aug 21, 2020

brodybits commented Aug 21, 2020

	type NamedTupleMember = Node;
	export interface NamedTupleMember extends Node {}

typecheck updates & fixes in src/language-js #8759

typecheck updates & fixes in src/language-js #8759

Conversation

brodybits commented Jul 15, 2020 • edited

alexander-akait left a comment

Choose a reason for hiding this comment

brodybits commented Jul 15, 2020

brodybits commented Jul 15, 2020 • edited

alexander-akait commented Jul 16, 2020

brodybits Jul 16, 2020

Choose a reason for hiding this comment

fisker commented Aug 12, 2020

brodybits commented Aug 12, 2020

brodybits commented Aug 13, 2020

ExE-Boss Aug 19, 2020

Choose a reason for hiding this comment

brodybits Aug 19, 2020

Choose a reason for hiding this comment

brodybits Aug 19, 2020

Choose a reason for hiding this comment

fisker commented Aug 21, 2020

brodybits commented Aug 21, 2020

brodybits commented Jul 15, 2020 •

edited

brodybits commented Jul 15, 2020 •

edited