search

microsoft / TypeScript

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

[Feature Request] Proposal for type annotations as comments #48650

Open matthew-dean opened 2 years ago

matthew-dean commented 2 years ago

Problem

  1. There are many authors that want to use TypeScript without a build step
  2. JSDoc is a verbose substitute for TypeScript and doesn't support all TS features.
  3. The above 2 problem statements are articulated in https://github.com/tc39/proposal-type-annotations, but it's unclear if that proposal will be accepted, and it would be a more limited subset of TypeScript. This could be implemented today, with a much lower lift.

Suggestion

This is not a new idea. It is a fleshed-out proposal for https://github.com/microsoft/TypeScript/issues/9694 and is also based on the prior art of https://flow.org/en/docs/types/comments/. #9694 was marked as "Needs Proposal" so this is an attempt at that proposal. (There is also prior art / similar conclusions & asks in the issue threads of the TC39 proposal.)

🔍 Search Terms

"jsdoc alternative", "jsdoc", "flotate", "flow comments"

✅ Viability Checklist

My suggestion meets these guidelines:

⭐ Proposal

A JavaScript file would be preceded by 

// @ts

The reason this is needed is to indicate the author's intent at

  1. Type-checking this JavaScript file as adhering to all of TypeScript's restraints (the TSConfig file), so a stronger version of @ts-check.
  2. Interpreting special comments as TypeScript types / blocks

Types of comment blocks:

Here's a basic example, borrowed from Flow:

// @ts

/*::
type MyAlias = {
  foo: number,
  bar: boolean,
  baz: string,
};
*/

function method(value /*: MyAlias */) /*: boolean */ {
  return value.bar;
}

method({ foo: 1, bar: true, baz: ["oops"] });

The TypeScript compiler would interpret /*: */ and /*:: */ as type annotations for TypeScript, making the entire JavaScript file a complete and valid TypeScript file, something that JSDoc does not provide.

Here are some other examples, borrowed from the TC39 proposal:

function stringsStringStrings(p1 /*: string */, p2 /*?: string */, p3 /*?: string */, p4 = "test") /*: string */ {
    // TODO
}

/*::
interface Person {
    name: string;
    age: number;
}
type CoolBool = boolean;
*/
//:: import type { Person } from "schema"

let person //: Person
// Type assertion
const point = JSON.parse(serializedPoint) //:: as ({ x: number, y: number })

// Non-nullable assertion - a little verbose, but works where JSDoc doesn't!
document.getElementById("entry")/*:: ! */.innerText = "..."

// Generics
class Box /*:: <T> */ {
    value /*: T */;
    constructor(value /*: T */) {
        this.value = value;
    }
}

// Generic invocations
add/*:: <number> */(4, 5)
new Point/*:: <bigint> */(4n, 5n)

// this parameter 
function sum(/*:: this: SomeType, */ x /*: number */, y /*: number */) {
    return x + y
}

// The above can be written in a more organized fashion like
/*::
type SumFunction = (this: SomeType, x: number, y: number) => number
*/
const sum /*: SumFunction */ = function (x, y) {
    return x + y
}

// Function overloads - the TC39 proposal (and JSDoc?) cannot support this
/*::
function foo(x: number): number
function foo(x: string): string;
*/
function foo(x /*: string | number */) /*: string | number */ {
    if (typeof x === number) {
          return x + 1
    }
    else {
        return x + "!"
    }
}

// Class and field modifiers
class Point {
    //:: public readonly
    x //: number
}

Important Note: an author should not be able to put any content in /*:: */ blocks. For example, this should be flagged as invalid:

/*::
function method(value: MyAlias): boolean {
  return value.bar;
}
*/

method({ foo: 1, bar: true, baz: ["oops"] });

Yes, the content of the /*:: */ is "valid TypeScript", but the engine should distinguish between type annotations / assertions from code that is to be available at runtime.

📃 Motivating Example

A lot of the motivations for this are the exact same as https://github.com/tc39/proposal-type-annotations; but this approach just solves it a different way, and could be done much sooner. The TypeScript engine would need to do little more than replace comment blocks in conforming .js files and then just immediately treat it as if it were a plain ol' TypeScript file.

💻 Use Cases

What do you want to use this for? This would allow teams / individuals / myself to use TypeScript without a build step! Gone would be "compile times" while developing.

What shortcomings exist with other approaches?

  1. The TC39 proposal is more limited than this proposal, for syntax space reasons
  2. JSDoc-based type-checking is more limited than this proposal, in that it doesn't support certain types, imports / exports, and as extensive of type-checking. This would support full type-checking of JavaScript.

What shortcomings exist with this approach?

  1. This is, of course, more verbose than plain TypeScript but it is, it should be noted, much less verbose than using JSDoc for typing (and would support all of TypeScript, unlike JSDoc).
  2. There would be, of course, some tooling support that wouldn't be present at first. For example, linters would need / want to be "TypeScript-aware", to lint the code within comment blocks. And code coloring / Intellisense should work in IDEs like VSCode to treat comment blocks like plain TypeScript. But I would anticipate support coming quickly from the community.
  3. The author would need to be aware that this is really just for type annotations. That is, one could not put any runtime TypeScript in /*:: */ because that would defeat the purpose. So there may be some initial confusion around usage. See the above example.

What workarounds are you using in the meantime? There are no current workarounds, to meet these particular goals. If you a) want to use all of TypeScript, b) don't want a build step in your JS files, there is no solution. Also, to again re-iterate the point, the TC39 proposal would also not meet these goals (like JSDoc, it also cannot support all of TypeScript), so there are benefits of doing this regardless of the outcome of that proposal.

Jack-Works commented 2 years ago

Another concise comemnt suggested in that proposal is:

//:: TypeA, TypeB => ReturnType
function f(a, b) {
    return a.handle(b)
}
lillallol commented 2 years ago

JSDoc is a verbose substitute for TypeScript and doesn't support all TS features.

JSDoc-based type-checking is more limited than this proposal, in that it doesn't support certain types, imports / exports, and as extensive of type-checking.

We have to make crystal clear on what kind of JSDoc we are talking about here. As far as I understand from what I read from your comments/issues you are talking about JSDoc without TS.

A currently available way of enabling static type checking in .js without the need to compile, is writing all your types in .ts files and then importing them in .js files via JSDoc comments.

AFAIK the only TS feature that is not supported like this, is enum. But is that an intrinsic inability of that way of enabling static type checking?

I would happily discuss with you about anything you think is un ergonomic, verbose, or lacking features regarding this way of static type checking. In my experience it is none of that.

and it would be a more limited subset of TypeScript.

Take a look on how they are suggesting to implement generics. They will introduce breaking changes.

There would be, of course, some tooling support that wouldn't be present at first.

It is already present with what I suggest.

matthew-dean commented 2 years ago

@Jack-Works That's interesting, and definitely //:: could be added, but I think the first step would be just essentially "escaping" valid TypeScript via comments.

Then, I think what TypeScript could add is the ability to type via function "overloading", such that this would become valid:

//:: function f(a: TypeA, b: typeB): typeA
function f(a, b) {
    return a.handle(b)
}

In other words, I think it's a much bigger ask if the existing code in comments is not valid TypeScript. Right now, in the proposal, it's basically drop-in search / replace (other than flagging certain TS as invalid in a comment, as noted).

matthew-dean commented 2 years ago

@lillallol

A currently available way of enabling static type checking in .js without the need to compile, is writing all your types in .ts files and then importing them in .js files via JSDoc comments.

You're right! There are clever workarounds. But writing types inline is often more self-documenting / easier to reason about, and JSDoc still wouldn't be as concise when it comes to assigning those types to vars / params. And, I think even with those, you're still missing some things when it comes to type-checking, although I can't remember what off the type of my head. That is, I think that @ts-check still has a lighter touch for a JSDoc file than an actual .ts file, IIRC.

Take a look on how they are suggesting to implement generics. They will introduce breaking changes.

Exactly. It can't be dropped in as-is. This proposal could.

matthew-dean commented 2 years ago

@lillallol From the TC39 proposal:

JSDoc comments are typically more verbose. On top of this, JSDoc comments only provide a subset of the feature set supported in TypeScript, in part because it's difficult to provide expressive syntax within JSDoc comments.

The motivation is the same.

lillallol commented 2 years ago

@matthew-dean

But writing types inline is often more self-documenting / easier to reason about

We should strive for separation of intent and implementation since it is a best practice.

But I have to admit that inline types are preferred when I create utility functions that I intend to use in multiple projects (lodash like utility functions), because like this I have to copy only a single thing from a file, and not two things (concretion and abstraction). Another use case for inline types is for defining simple types (e.g. boolean, number etc) for some variables that are declared inside functions (my projects are just functions and singletons), but again when the types get more involved I add them in .ts files.

For non lodash like projects, I usually write all the types into these files:

I think you will also find that self explanatory and easy to reason about.

If you want to see the types of a concretion then you can hover over it and VSCode will show the type. If you are still not satisfied with what VSCode shows then you can ctrl+click on the imported type for VSCode to go to its definition.

JSDoc comments are typically more verbose.

This argument is not a real concern. At least in my own experience (e.g. the projects I have done). Do you really actually press more the keyboard when using JSDoc? If yes then how much? 1 key? 2 keys? You wanna get rid of those taps and introduce a new comment syntax? Is it worth for the extra fragmentation it will create? If you (and a substantial amount of people) have created projects (10k+ lines of code) with the way of static type checking I suggest, and you find these extra taps reduce DX, then fine I am with you.

Strictly speaking, when someone sticks to separation of intend and implementation, importing types via JSDoc is not necessarily more verbose when compared to writing everything in .ts files. In fact sometimes it can be less verbose.

Regarding which is more readable, I have to say that this is a matter of what someone has gotten used to. For example initially I found ts code not as readable as js code. But then I got used to it. Same for the importing types via JSDoc.

And, I think even with those, you're still missing some things when it comes to type-checking, although I can't remember what off the type of my head.

Just do not ask me how to type classes, because I have no clue about classes. Still the same question remains:

is that an intrinsic inability of that way of enabling static type checking?

or is it something that can be supported in the future?

matthew-dean commented 2 years ago

@lillallol

I'm confused where you are coming from or what your argument is. In no way would what I'm (or others) proposing have a negative impact on JSDoc-style typing. If it works for you in a way that matches your development workflow, great. That's not everyone's experience, and I think it's clearly articulated by even people on the TypeScript team that the JSDoc flow is not the greatest experience, from their perspective.

So, if this doesn't match a need you have, that's okay. This isn't for you. Just like, if someone is fine with transpiling TypeScript / having a build step, this isn't for them either. But it's a clearly articulated need by other developers. This would be an alternate style of embedding types that would be compatible with the existing TypeScript ecosystem, including JSDoc-typed files.

lillallol commented 2 years ago

That's not everyone's experience

Lets be more specific with examples here.

I think it's clearly articulated by even people on the TypeScript team that the JSDoc flow is not the greatest experience

What do you mean by JSDoc flow? You mean the way I suggest? If yes then I would like to have some links, or at least if ts maintainers see that, have a discussion on that, here.

But it's a clearly articulated need by other developers.

There is already a solution for that need, which actually promotes best practices (separation of intent and implementation) rather than embracing bad practices (embracing of mixing intent with implementation). From what you suggest we end up hard coding .js files with ts. This is not done with the way I suggest : /**@type {import("some/path/without/ts/extension").IMyType}*/. That path can refer to .ts or a flow file or whatever. Like this you can make your code base work for any type system without having to change the .js or .ts files.

If your response is :

but this bad practices is already supported by ts compile to js method

then I would like to make myself crystal clear on that one: compiling ts to js as inferior way of developing to what I suggest (read here for more).

simonbuchan commented 2 years ago

Teeny note, there's already // @ts-check, or --checkJs. I think it's probably safe enough to just use those (who has a critical need to not change existing comments with leading :: but also run with --checkJs?)

I suspect this would be quite simple to design, add and document, especially with prior art, and few downsides (let me know if I'm wrong!)

It's not even all that much in conflict with the ECMA proposal, which can have significantly nicer experience with inline annotations.

dfabulich commented 2 years ago

When Flow first introduced its streamlined comment syntax, they imported it wholesale from flotate, a piece of software built pretty much entirely by one person.

Rather than a written proposal, I think what's needed here is a working prototype. I think it makes sense to experiment with Flow's syntax, and even to build on it with //: and/or //:: syntax. Try it out in a real project and see how it feels!

(I wonder whether //:: would feel better inside the function declaration, like docstrings in Python.)

function f(a, b) {
    //:: TypeA, TypeB => ReturnType
    return a.handle(b)
}
simonbuchan commented 2 years ago

@dfabulich one of the things I like about the Flow behavior is it's extremely straightforward: just remove the wrapping /*:: and */ to get the code Flow sees. I wouldn't want to mess with that property.

I actually took a stab already at adding it to a fork of typescript. Initially, seemed pretty easy to add to scan() in scanner.ts, but I haven't yet figured out how to get the parser to not try to parse the closing */ as an expression, so it gets very confused. I assume some .reScan*() method is kicking in and breaking everything, but maybe I also need to do skipTrivia(). Any compiler devs that are bored and have a guess, let me know!

There is an interesting issue though: the following code would seem to be valid, but give different results when run through the transpiler from directly:

let runtime = "native";
/*::
runtime = "transpiled";
*/
console.log(runtime);

Not sure if that's a bug or a feature!

dfabulich commented 2 years ago

Yeah, I just think it's a hassle to add four characters (/* */) for each function parameter and the return type (or maybe six if you include spaces around the comment delimiters):

function method(a /*: number */, b /*: number */, c /*: number */, ) /*: number */ {
  return a + b + c;
}
function method(a, b, c) {
  //: number, number, number => number
  return a + b + c;
}
matthew-dean commented 2 years ago

@simonbuchan

Teeny note, there's already // @ts-check, or --checkJs

So there is a very low but important risk if you left this as is -- it's possible that someone is using //: or /*: in a comment start. So I think it's important to "flag" this new comment behavior. Essentially I would propose that // @ts is a "superset" of // ts-check. It's TypeScript checking plus escaping this comment syntax. In addition, you have to consider that JSDoc types and this form of type-checking could potentially be in conflict, if an author defines both, so I would propose that // @ts "ignores" JSDoc types if they are present and just treats them as comments. (Unless it's trivial to just say that this comment form would "win" if there are two type indicators.)

@dfabulich

Essentially, I feel you're not wrong (although I still disagree with this syntax as not very TS-y or JS-y); I just feel it's a huge mistake to conflate these two things in one feature, as it's a much bigger ask. These should be two different proposals.

  1. This proposal (denoting valid types in comments) -- essentially supporting currently valid TypeScript
  2. After / if proposal #1 is accepted / has feedback, is in use, adding "simplified forms" to types-in-comments. (This should be an entirely different proposal). This needs a lot more iteration and would be a much harder push because it would be potentially adding code in "escaping" comments that would not currently be valid TypeScript / JavaScript.
matthew-dean commented 2 years ago

@simonbuchan As to this:

let runtime = "native";
/*::
runtime = "transpiled";
*/
console.log(runtime);

Not sure if that's a bug or a feature!

This should definitely be treated as a bug / type-checking error by TypeScript, and IMO this is a bug if Flow supports that. The resulting code is runnable but the result is not expected. So ideally, only "typing" forms would be allowed within comments.

And definitely this code should be entirely thrown out (throw an error):

/*::
let runtime = "transpiled";
*/
console.log(runtime);

(TypeScript should throw an error as runtime being undefined.)

When I have time, I can refine the proposal with specifying what is allowed / disallowed in these comment blocks, and not just escaping "any TypeScript".

somebody1234 commented 2 years ago

I've been working on an implementation of this for a while: TypeScript TypeScript-TmLanguage Note that you'll need to clone the vscode repository, change the url of the script that updates the typescript (and by extension, javascript) .tmLanguages, and finallly add the javascript/ folder as an extension, to get highlighting to work

Note also that it's extremely WIP and still has print debugging statements (D:) - there are still quite a number of issues, especially (or mostly?) with incremental parsing

Of note is that runtime statements (mostly) error as expected; enums and const enums are completely banned; declare should mostly error/pass as expected too however it seems to not work in some situations like this:

/*::declare const FOO: unique symbol;*/

(Forgot to mention - for TypeScript do npm run gulp watch-local and point your typescript.tsdk vscode setting to /path/to/cloned/TypeScript/built/local)

matthew-dean commented 2 years ago

@somebody1234 Awesome! I still think it needs documentation of the technical details -- how / what / which things are allowed, so I want to add that to this proposal. Are there any other things you've caught in the implementation that need to be considered?

somebody1234 commented 2 years ago

(Note: For reference, there's the test file I use at the bottom. It should contain all the things that are allowed) (Note 2: For parsing, most runtime constructs are parsed in types-as-comments - the diagnostics that they are not allowed in types-as-comments are added later)

Misc notes

What is allowed

function foo();
Test file 
/*::
type Left = 1;
type Right = 2;
type Test = Left | Right;
type Test2 = Left | Right;
let invalid: Test;
*/
type Invalid = 1;

let annotation/*: string*/ = 'a';
/* normal comments should work fine */
export function fromString(s/*: string */) {
  //
}

export function toString() {
  //
}
let foo/*: number */;

const testNestedAsConst = {
  arr: [1, 2, 3] /*:: as const */
};
const TokenType = {
  CRLF: 0,
} /*:: as const */;
/*::
type TokenType = (typeof TokenType)[keyof typeof TokenType];
type Node =
| { type: typeof TokenType.CRLF; };
*/

const asConstTest = 1 /*:: as const */;

function argsTest(s/*:: : string */, s2/*:: ?: string */) {
  return 0;
}

function thisParamTest(/*:: this: number, */ a/*: string */) {
  return 0;
}

function thisParamTest2(/*:: this: number*/ a/*: string */) {
  return 0;
}

function thisParamTest3(/*:: this: number */) {
  return 0;
}
let foo1 /*: number */;
let testTheThing/*: number */;
// let testTheThing2/*:: : */
// let fn = fnGeneric/*::*/('ok');
class ClassIndex1 {
  /*::[k: string]: string;*/
}
class ClassMember1 {
  a/*: string */
}
class ClassMember2 {
  a/*: string */;
}
class ClassMember3 {
  a/*:: : string */
}
class ClassMember4 {
  a/*:: : string */;
}
// let a: ;
// let a /*:  */;
class ClassMember5 {
  a/*:: ?: string */
}
class ClassMember6 {
  a/*:: ?: string */;
}
class ClassMember7 {
  a/*:: !: string */
}
class ClassMember8 {
  a/*:: !: string */;
}
class ClassMembers1 {
  a/*: string */
  b/*: string */;
  c/*: string */
}
class ClassMembers2 {
  /*::[k: string]: string;*/
  a/*: string */;
  b/*: string */
  c/*: string */;
}
class ClassDeclare1 {
  /*::
  [k: string]: string;
  declare a: '1';
  declare b: '2';
  c: '3';
  */
}

class ClassGet1 {
  get a() { return 1; }
  set a(v) {}
  static get a() { return 1; }
  static b = 1;
  // /*:: static */ a = 1;
  private get c() { return 1; }
  /*:: private */ get d() { return 1; }
  /*:: public */ static set e(v/*: number */) {}
}
let classGet1/*:: !: ClassGet1 */;
classGet1.d;

class ClassModifiers1 {
  /*:: public */a/*: number */;
}

class ClassModifiersInvalid {
  /*:: public static*/ public a/*: number */;
}

class ClassMisc {
  bar/*::?*/()/*: number*/ { return 0; }
  get foo(): number { return 0; }
}

class ClassPropertyConstructor {
  constructor(/*:: public */a/*: number */) {
    //
  }
}

const what = { foo!: 1 };

/*::
export = asConstTest;

interface I {}

type Wot = {
  private a: number;
  // private b/*: number*/;
  // /*:: private */a/*: number */;
};
declare function declareTest(): void;
*/
declare function declareTestBad()/*: void*/;
/*:: declare */ let a: number;

// TODO: make sure all typescript syntax still correctly errors in js
class ClassExtendsImplements extends String /*:: implements I */ {
}
class ClassImplements /*:: implements I */ {}

class ClassGeneric/*::*/ {
  a/*: T */;
  b/*: [T] */;
  c/*::*/() {}
  d/*::?*/() {}
}
class ClassModifier {
  /*:: private*/ bar/*: string*/
}
let letGeneric1/*: ClassGeneric<1> */;
let letGeneric2/*:: !: ClassGeneric<1> */;
function fnGeneric/*::*/(it/*:T*/) {
  return it;
}

/*::
declare module "./types_as_comments_test" {
  type A = 'A';
  type B = 'B';
  type C = 'C';
  type D = 'D';
  type E = 'E';
  type F = 'F';
}
*/

import { /*:: type A,*/ } from "./types_as_comments_test"
import { B, /*:: type C,*/ } from "./types_as_comments_test"
import { D, /*::type E, type F */ } from "./types_as_comments_test"
export { /*:: type A as A_*/ }
export { B as B_, /*::type C as C_*/ }
export { D as D_, /*::type E as E_, type F as F_ */ }
function thisParamIdk(/*:: this: Foo3 */) {}
class Foo3 {
  /*::declare x: Foo3;*/
  // TODO: this errors
  // /*::declare y: Foo3;;*/
}
class Foo4 {
  x/*: Foo4 */
}

fnGeneric/*::<[1, String, () => {}, (a: 1, ...b: any[]) => asserts a is any, (a:number)=>a is 1, number, {[k:1]:2;}]>*/();

// const instantiationExpression = [].map/*::*/;
const nonNullAssertion = 1/*::!*/;
const genericArrowFunction = /*::*/() => {};
const genericAsyncArrowFunction = async /*::*/() => {};
// note that these two are invalid
// const genericSimpleArrowFunction = /*::*/ foo => {};
// const genericSimpleAsyncArrowFunction = async /*::*/ foo => {};
const prefixTypeAssertion = /*::*/ 1;

/*::
declare interface AAAAA {
  bar: 1;
}
*/
Jack-Works commented 2 years ago

anyone knows of any other constructs that are exclusive to TypeScript please let me know

namespace / module

namespace X {
    const a = 1
}

import =

import react = require('react')
somebody1234 commented 2 years ago

import = seems to be correctly handled currently - namespace/module don't though, nice catch

the fix is simple though (and it's relatively minor) so i'll hold off on committing it for now (if you want it asap though, search for Namespace (+ Module) in src/compiler/program.ts)

somebody1234 commented 2 years ago

Ah... also worth noting that a bunch of diagnostic messages will be '240' expected or similar... for print debugging reasons. Those normally say "Comments as types cannot contain JavaScript".

On that note, feel free to suggest improvements to the new diagnostic messages as well

simonbuchan commented 2 years ago

Interesting approach, seems like a lot of work! I was thinking banning bad uses would be done with something like a TokenFlag that gets bubbled up to something that definitely knows if it could be emitted or not, but I didn't get very far so maybe that would be a problem.

somebody1234 commented 2 years ago

Does seem like a lot of work - but I think it's relatively low effort compared to the alternatives. Plus this way you get a huge amount of control over where exactly they're valid, error recovery etc. Especially compared to a simple find-and-replace. Not to mention find-and replace would replace in strings too, etc etc

Offtopic but, one concern would be, it adds quite a bit of complexity to parser logic so it might cause performance issues - however I'm guessing it's not that bad since most of the time is probably usually typecheck time anyway

simonbuchan commented 2 years ago

The profiles I've seen are all dominated by FS access, binding and type checking, but with JavaScript you can always fall off a performance cliff and have some code run 100x worse. You'd probably notice the tests being noticably slower though!

Not sure exactly what you mean by find and replace - doing that before parse would break location reporting along with all the other issues! My attempt was to get the scanner to treat /*:: and a matching */ as separate comment trivia, so the parser didn't need to pay any attention. Obviously not a complete solution, if you don't want to accept emittable code, but I felt that could be in a relatively simpler diagnostic visitor pass after parsing.

simonbuchan commented 2 years ago

But again, I've only spent like an hour on this!

somebody1234 commented 2 years ago

find and replace as in, replacing /*:: with three spaces; */ with two. which should break exactly zero location reporting

treat /*:: and a matching */ as separate comment trivia

That was, in fact, my initial approach, but it's just (IMO) awful for checking that, y'know:

  • things that should be in type comments are actually in type comments
  • things that shouldn't, are actually not in type comments
  • type comments pair up in reasonable places - i.e. no /*:: class Foo */
  • type comments are balanced at all etc etc

In other words - AFAICT it'd make parsing almost trivial, but getting diagnostics at all, let alone correct ones, will be much, much harder

as for "have some code run 100x worse" - i don't think so here. but there's like, say, up 2x the work done in certain parts so there's potential of, say, 10% (or more?!) worse performance since everywhere it checks for a type annotation it must also check for /*: and /*:: now

jespertheend commented 2 years ago

I'm especially keen on

// Generic invocations
add/*:: <number> */(4, 5)
new Point/*:: <bigint> */(4n, 5n)

as far as I know there's no way to do this with JSDoc at the moment.

lillallol commented 2 years ago

@jespertheend

as far as I know there's no way to do this with JSDoc at the moment.

Sorry but I do not fully understand what you are trying to do with the example you provide. Can you provide a type definition for add and Point?

jespertheend commented 2 years ago

The example was taken from the first comment. Basically in TypeScript when a function has a generic parameter:

function add<T>(a: T, b: T) : T {
  // do something
  return {} as T;
}

You can simply invoke it like

add<number>(2, 3);

With JavaScript and JSDoc this is not possible.

Normally this is not a big deal since the generic parameter will be implicitly inferred by the types you pass in for a and b, but I'm running into an issue right now where I want to make sure my types are correct for the assertEquals(actual, expected) function in deno_std. But the function has been overloaded with an extra signature that doesn't contain the generic parameter. So if you want to make sure the values you pass in for actual and expected have the same type, you have to either use TypeScript or write a wrapper function for assertEquals().

lillallol commented 2 years ago

With JavaScript and JSDoc this is not possible.

This is a common misconception that is not valid. Here is how to do it.

For some strange reason I am not getting the errors :

  • Variable 'token1' is used before being assigned.
  • Variable 'token2' is used before being assigned.

in the playground (I am too lazy to find why) although I do get them in vscode.

I think it is a matter of support from the types system to enable a built in type function (e.g. token<> and untoken<>) to make clear our intentions to the type system so such errors not occur.

Edit : By the way you provided the solution your self, without the extra parameter solution I provided.

somebody1234 commented 2 years ago

seems like its because strictNullChecks is off by default in playground js: Playground

trusktr commented 8 months ago

@lillallol the extra variables in your JSDoc example for @jespertheend are very superfluous. We definitely need the concise syntax.

Yeah, I just think it's a hassle to add four characters (/* */) for each function parameter and the return type (or maybe six if you include spaces around the comment delimiters):

function method(a /*: number */, b /*: number */, c /*: number */, ) /*: number */ {
  return a + b + c;
}
function method(a, b, c) {
  //: number, number, number => number
  return a + b + c;
}

From an end user perspective, @dfabulich's second example is simpler, cleaner, less noisy. I acknowledge it would be more work than essentially only ignoring comment markers, but the end user experience could be improved.

What about documentation?

It feels like we should also consider alternative documentation comment syntax to go along with the new type comment syntax, otherwise mixing the two doesn't seem to accomplish much. Here's an example with JSDoc comments, but new type comment syntax:

/**
 * @param a - description for a
 * @param b - description for b
 * @param c - description for c
 */
function method(a, b, c) {
  //: number, number, number => number
  return a + b + c;
}

Here's an with both new doc and type comment syntax:

function method(a, b, c) {
  //: number, number, number => number
  //: a - description for a
  //: b - description for b
  //: c - description for c

  let foo = 123 // is this affected by the previous comment or not? 123 not assignable to function?

  return a + b + c;
}

Alternative:

//: a:number - description for a
//: b:number - description for b
//: c:number - description for c
function method(a, b, c) {
  let foo = 123
  return a + b + c;
}

Here are more examples of potential doc+type comments:

/** :string|boolean - the description */
let foo = "foo"
foo = false

or

// :string|boolean - the description
let foo = "foo"
foo = false

Suppose the strip-markers stuff still all works,

function foo(a /*:number*/, b /*:string*/) /*:boolean*/ {...}

but with alternatives like above and

function foo(
  a, //:number
  b, //:string
) { //:boolean
  ...
}

or a combo, depending on taste:

function foo(
  a, //:number
  b, //:string
) /* :boolean */ {
  ...
}

Or

function foo(
  a, //:number - with description 
  b, //:string - with description
) { //:boolean - with description
  ...
}

Similar to one of my above examples, but with an explicit return type+doc:

//: a:number - description for a
//: b:number - description for b
//: c:number - description for c
//: :string - description of return value
function method(a, b, c) {
  return "Val:"+a + b + c;
}

Maybe we don't need some of the colons:

// a: number - description for a
// continued on multiple lines with a code sample:
// ```js
// method(a,b,c)
// ```
// b: number - description for b
// c: number - description for c
// :string - description of return value
function method(a, b, c) {
  return "Val:"+a + b + c;
}

Depending on the code formatting (new lines or not, etc), and on whether or not documentation is needed, the desirable format to use can vary, so a few different ways of commenting could be beneficial.

Having a more concise type comment syntax, but having to fall back to JSDoc for documentation, leaves some sort of awkward feeling.

The more concise this can be, the better (for us end users).

dfabulich commented 8 months ago

I recently learned that JSDoc syntax already has a somewhat more concise form.

Instead of this:

/**
 * 
 * @param {number} a 
 * @param {number} b 
 * @param {number} c 
 * @returns number 
 */
function method(a, b, c,) {
    return a + b + c;
}

You can write this:

/** @type {(a: number, b: number, c: number) => number} */
function method(a, b, c,) {
    return a + b + c;
}

It works today, out of the box with TypeScript.

Here's a more complex example. You can turn this:

/**
 * @typedef {Object} MemberGroupsResponse
 * @property {Object.<string, MemberGroup>} memberGroups
 * @property {Object.<string, Array.<string>>} memberGroupsByUser
 *
 * @function getMemberGroupsUncached
 * @async
 * @param {string} month
 * @param {string} memberGroupsSheet
 * @returns {Promise.<MemberGroupsResponse>}
 */
async function getMemberGroupsUncached(month, memberGroupsSheet) { /* ... */ }

… into this:

/**
 * @type {(month: string, memberGroupSheet: string) => Promise<{
 *   memberGroups: Record<string, MemberGroup>;
 *   memberGroupsByUser: Record<string, string[]>;
 * }>}
 */
async function getMemberGroupsUncached(month, memberGroupsSheet) { /* ... */ }

Still, improvement here is possible. It'd be great if there were a /*: ... */shorthand for /** @type {...} */. That'd be especially useful for JSDoc Casts.

/**
 * @type {number | string}
 */
var numberOrString = Math.random() < 0.5 ? "hello" : 100;
var typeAssertedNumber = /** @type {number} */ (numberOrString);

Could be:

/*: number | string */
var numberOrString = Math.random() < 0.5 ? "hello" : 100;
var typeAssertedNumber = /*: number */ (numberOrString);

Today, when you want to use TS as const in JSDoc, you have to do it like this:

let one = /** @type {const} */(1);

It'd be great to do it like this:

let one = /*: const */(1);
trusktr commented 8 months ago

You can write this:

/** @type {(a: number, b: number, c: number) => number} */
function method(a, b, c,) {
    return a + b + c;
}

Yes indeed, I am aware of using TS syntax inside JSDoc syntax, and I do take advantage of it, but there's one particular situation where it falls apart:

We cannot add documentation at all when using that format. For example, try converting this one over:

/**
 * 
 * @param {number} a - description for a
 * @param {number} b - description for b 
 * @returns number - description for return value
 */
function method(a, b) {
    return a + b;
}

We need a concise syntax for documentation and types.

var typeAssertedNumber = /** @type {number} */ (numberOrString);
var typeAssertedNumber = /*: number */ (numberOrString);

Indeed that one has been annoying, and your idea there is a lot better. Another possibility is:

var typeAssertedNumber = numberOrString /*: as number */;
// or
var typeAssertedNumber = numberOrString //: as number
trusktr commented 8 months ago

Is it possible to prototype this comment type system as a TypeScript transform plugin? Ideally the prototype would have:

  • making types work in TS, as above (obvious one)
  • IDE support (f.e. VS Code tooltips showing inline documentation)
  • doc generator that can extract docs (as from my above examples) and produce basic HTML output as a starting point to be able to transition from JSDoc-based tools for documentation generation.
lillallol commented 8 months ago

Yes indeed, I am aware of using TS syntax inside JSDoc syntax, and I do take advantage of it, but there's one particular situation where it falls apart:

We cannot add documentation at all when using that format.

That is not true. You write the type in a ./privateApi.ts file like this:

/**
 * some description about what this function does (that is actually what it
 * returns)
*/
export declare function method(
    /** description for a*/
    a: number, 
    /** description for b*/
    b: number,
) : number;

and then import it in a .js file like this:

/**
 * @type {import("./privateApi.js").method} 
*/
function method(a, b) {
    return a + b;
}

//hover over method and VSCode will popup a window that have both the types and description
method(,)//the comma will trigger the popup with description for parameters in VSCode

If you attempt to do the same thing, using arrow functions (both in .ts and .js), it will not work. It is a matter of support from the typescript team to make it work for arrow functions as well.

doc generator that can extract docs (as from my above examples) and produce basic HTML output as a starting point to be able to transition from JSDoc-based tools for documentation generation.

Notice that we achieved to write both the type and the JSDoc description in a single place, without the implementation. That is all we need for documentation. We do not actually need documentation generation libraries, they are an anti pattern, as it can been seen from the following issue I opened in typedoc [link].

Regarding type as assertions I do consider them an anti pattern since they override the type checker and can be replaced with run time assertions. Const assertions can be made using functions calls nowadays so no extra syntax is required for them inside JSDoc in .js files.

I have created projects in which all types are defined in .ts and are imported in .js files via /**@type {import("./path/to/file.js")}*/. The argument of verbosity has never been an actual issue for me.

To sum up I think that there is no need to implement any of the changes suggested in the context issue.

trusktr commented 8 months ago

That is not true. You write the type in a ./privateApi.ts file like this:

That is possible indeed, but I think some people prefer not to split to separate files, which is a valid desire. So my comment was correct, when staying in the same file.

lillallol commented 8 months ago

some people prefer not to split to separate files

I can accept that as a valid argument since we are in the typescript repo and not the type annotations proposal repo.

msadeqhe commented 8 months ago

How do you separate the implementation? When the programmer want to use type checking while using a library?

/**
 * @type {import("./privateApi.js").method} 
 */

function method(a, b) {
    let x: number = a + b;
    let y: number = a * b;
    return x / y;
}
msadeqhe commented 8 months ago

How do you separate the implementation? When the programmer want to use type checking while using a library?

/**
 * @type {import("./privateApi.js").method} 
 */

function method(a, b) {
    let x: number = a + b;
    let y: number = a * b;
    return x / y;
}

If we separate the whole implementation to .ts file, then we're not using JavaScript.

lillallol commented 8 months ago

How do you separate the implementation? When the programmer want to use type checking while using a library?

If we separate the whole implementation to .ts file, then we're not using JavaScript.

I do not understand what you are talking about. Can I see more detailed examples please.

msadeqhe commented 8 months ago

I mean your assumption that "we don't need type annotation in JS because we can put type annotation in separate .ts file and import it in .js file", is wrong.

Because in your solution, only type annotation for function declarations can be imported to JS, but most of the times, we are using type annotation in function body. That's why we need to be able to use type annotation in JS files. Of course if we put the function body in .ts file, we have to transpile it from TS to JS, that's why your solution doesn't work.

lillallol commented 8 months ago

only type annotation for function declarations can be imported to JS

That is not valid. You can import any type from .ts files to annotate statements.

/**
 * @type {import("./privateApi.js").IMethod} 
 */
function method(a, b) {
    /**
     * No need to use that annotation anyway since TS infers types.
     * @type {import("./privateApi.js").INumber}
     */
    let x = a + b;
    /**
     * No need to use that annotation anyway since TS infers types.
     * @type {import("./privateApi.js").INumber}
     */
    let y = a * b;
    return x / y;
}
//./privateApi.ts
export type IMethod = (a : number, b : number) => number;
export type INumber = number;

Regarding the type annotations proposal the enforcement of separation of implementation and abstractions in different files enables someone to standardise type-import annotations without the need to define a type system and simultaneously not restricting its syntax. That is exactly the proposal I want to present to tc39.

msadeqhe commented 8 months ago

OK. How do you annotate the type of x and y variables?

/**
 * @type {import("./privateApi.js").IMethod} 
 */
function method(a, b) {
    /**
     * No need to use that annotation anyway since TS infers types.
     * @type {import("./privateApi.js").INumber}
     */
    let x = a + b;
    /**
     * No need to use that annotation anyway since TS infers types.
     * @type {import("./privateApi.js").INumber}
     */
    let y = a * b;
    return x / y;
}

This is a simple example, but in complicated function bodies, we need a direct way to use TypeScript features within function bodies.

lillallol commented 8 months ago

This is a simple example, but in complicated function bodies, we need a direct way to use TypeScript features within function bodies.

Please give an example.

lillallol commented 8 months ago

Here is one. But then again it is a matter of support, i.e. not an intrinsic inability of the way of static typing I suggest.

msadeqhe commented 8 months ago

This is an example from VSCode repository:

public selectToBracket(selectBrackets: boolean): void {
    if (!this._editor.hasModel()) {
        return;
    }

    const model = this._editor.getModel();
    const newSelections: Selection[] = [];

    this._editor.getSelections().forEach(selection => {
        const position = selection.getStartPosition();
        let brackets = model.bracketPairs.matchBracket(position);

        if (!brackets) {
            brackets = model.bracketPairs.findEnclosingBrackets(position);
            if (!brackets) {
                const nextBracket = model.bracketPairs.findNextBracket(position);
                if (nextBracket && nextBracket.range) {
                        brackets = model.bracketPairs.matchBracket(nextBracket.range.getStartPosition());
                }
            }
        }

        let selectFrom: Position | null = null;
        let selectTo: Position | null = null;

        if (brackets) {
            brackets.sort(Range.compareRangesUsingStarts);
            const [open, close] = brackets;
            selectFrom = selectBrackets ? open.getStartPosition() : open.getEndPosition();
            selectTo = selectBrackets ? close.getEndPosition() : close.getStartPosition();

            if (close.containsPosition(position)) {
                // select backwards if the cursor was on the closing bracket
                const tmp = selectFrom;
                selectFrom = selectTo;
                selectTo = tmp;
            }
        }

        if (selectFrom && selectTo) {
            newSelections.push(new Selection(selectFrom.lineNumber, selectFrom.column, selectTo.lineNumber, selectTo.column));
        }
    });

    if (newSelections.length > 0) {
        this._editor.setSelections(newSelections);
        this._editor.revealRange(newSelections[0]);
    }
}

newSelection, selectFrom and selectTo have type annotations. How do you put the declaration of them to a separate .ts file? How do you import them to .js file? They are local variables.

msadeqhe commented 8 months ago

What about local functions and local types? If we put all of them to a .ts file, it would resemble spaghetti coding style.

lillallol commented 8 months ago

This is not valid .js nor .ts. Can I have minimal reproducible example? Nvm:

   /**@type {import("./some/place.js").ISelectToBracket}*/
   selectToBracket(selectBrackets) {
    if (!this._editor.hasModel()) {
        return;
    }

    const model = this._editor.getModel();
    const newSelections: Selection[] = [];

    this._editor.getSelections().forEach(selection => {
        const position = selection.getStartPosition();
        let brackets = model.bracketPairs.matchBracket(position);

        if (!brackets) {
            brackets = model.bracketPairs.findEnclosingBrackets(position);
            if (!brackets) {
                const nextBracket = model.bracketPairs.findNextBracket(position);
                if (nextBracket && nextBracket.range) {
                        brackets = model.bracketPairs.matchBracket(nextBracket.range.getStartPosition());
                }
            }
        }
                /**@type {import("./from/somewhere.js").IPostionNull}*/
        let selectFrom  = null;
                /**@type {import("./from/somewhere.js").IPostionNull}*/
        let selectTo = null;

        if (brackets) {
            brackets.sort(Range.compareRangesUsingStarts);
            const [open, close] = brackets;
            selectFrom = selectBrackets ? open.getStartPosition() : open.getEndPosition();
            selectTo = selectBrackets ? close.getEndPosition() : close.getStartPosition();

            if (close.containsPosition(position)) {
                // select backwards if the cursor was on the closing bracket
                const tmp = selectFrom;
                selectFrom = selectTo;
                selectTo = tmp;
            }
        }

        if (selectFrom && selectTo) {
            newSelections.push(new Selection(selectFrom.lineNumber, selectFrom.column, selectTo.lineNumber, selectTo.column));
        }
    });

    if (newSelections.length > 0) {
        this._editor.setSelections(newSelections);
        this._editor.revealRange(newSelections[0]);
    }
}

it would resemble spaghetti coding style.

There is no need to use a single file for your types. I usually use these:

  • publicApi.ts is used to define the public API
  • privateApi.ts is used to define the private API
  • types.ts is used to define types used by the abstractions and implementations of the private API
  • testApi.ts is used to define types that are used only in test files

I never had a problem with spaghetti code.

trusktr commented 8 months ago

@lillallol Here's a simple repo with the first commit written in TypeScript, npm i && npm start installs typescript, builds to JS, opens a browser tab, logs to console.

https://github.com/trusktr/buildless-typescript

What's the best way (in your opinion) to convert it to buildless (keep only the static server) while keeping the code as simple as possible? Can you make a PR so we can see the diff?

EDIT, nevermind, I updated it to JS with no build. That one wasn't so bad! Better to not have separate files for the classes in that case.

Here's one class:

/**
 * @abstract
 * @template {object} T
 */
export class Foo {
  foo = "456"

  /**
   * @abstract
   * @returns {T}
   */
  method() {
    throw "subclass must implement"
  }

  doFoo() { this.foo; this.method() }
}

@abstract doesn't work yet (feature request). Here's the other class:

import { Bar } from "./Bar.js"
import { Foo } from "./Foo.js"

const FooBar = /** @type {typeof Foo<Bar>} */ (Foo)

export class Test extends FooBar {
  /** @override */
  method() {
    return new Bar()
  }
}

const b = new Test().doFoo()
b.logBar()

with terser comments, it could be:

export /*: abstract */ class Foo/*:<T extends object>*/ {
  foo = "456"

  /*: abstract */method() { //: T
    throw "subclass must implement"
  }

  doFoo() { this.foo; this.method() }
}
import { Bar } from "./Bar.js"
import { Foo } from "./Foo.js"

export class Test extends Foo/*:<Bar>*/ {
  /* :override */ method() {
    return new Bar()
  }
}

const b = new Test().doFoo()
b.logBar()

That still feels a little awkward and noisy to me. Here's an alternative that I like more:

//: abstract
export class Foo { //:<T extends object>
  foo = "456"

  //: abstract
  method() {} //: T

  doFoo() { this.foo; this.method() }
}
import { Bar } from "./Bar.js"
import { Foo } from "./Foo.js"

export class Test extends Foo { //:<Bar>
  //: override
  method() {
    return new Bar()
  }
}

const b = new Test().doFoo()
b.logBar()

Ideas are varied at the moment, but I think that last one is cleanest so far.

There would need to be rules that associate the comments to the parts they annotate. For example, the //: T annotates method's return value because that's the last thing on the same line that it can (say, we're not annotating the curly braces, but the function)

SIde by side:

/**
 * @abstract
 * @template {object} T
 */
export class Foo {
  foo = "456"

  /**
   * @abstract
   * @returns {T}
   */
  method() {
    throw "subclass must implement"
  }

  doFoo() { this.foo; this.method() }
}
//: abstract
export class Foo { //:<T extends object>
  foo = "456"

  //: abstract
  method() {} //: T

  doFoo() { this.foo; this.method() }
}
theScottyJam commented 2 weeks ago

I ended up spending some time working on this problem, and was able to put together a fork of TypeScript that allows TypeScript syntax to be inside of comments - it took more work than I thought it would :p. Anyways, it's up one NPM and GitHub.

As for putting JSDocs inside of a "TS Comment" (What I'm calling these /*:: ... */ /*: ... */ things), I ended up solving that issue by just having you close both the JSDoc comment and TS comment with the same closing comment token, then right afterwards you would re-open the TS comment, like this:

/*::
interface User {
  readonly username: string
  readonly birthday: Date
  /** @deprecated *//*::
  readonly age: number
}
*/

It's a little funky, but it works good enough for now.

  • © Githubissues.
  • Githubissues is a development platform for aggregating issues.