microsoft / TypeScript

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

Allow to explicitly pass type parameters via JSDoc #27387

Open ifeltsweet opened 5 years ago

ifeltsweet commented 5 years ago

Search Terms

jsdoc generics type parameters constraints

Suggestion

It seems like it is not possible via JSDoc to explicitly tell compiler which type parameters to pass to a generic function.

Use Cases

In TypeScript it is possible to explicitly pass type parameters such as mongoose.model<Model, Schema>('modelName', Schema) while I could not find a way to do same with JSDoc.

Examples

mongoose.model has two signatures, first one with one type parameter and the other with two. To make use of the second signature we must pass types explicitly.

export function model<T extends Document>(
  name: string,
  schema?: Schema,
  collection?: string,
  skipInit?: boolean
): Model<T>;

export function model<T extends Document, U extends Model<T>>(
  name: string,
  schema?: Schema,
  collection?: string,
  skipInit?: boolean
): U;
//  Choose second signature in typescript.
const model = mongoose.model<Model, Schema>('modelName', Schema);

// With JSDoc, compiler always chooses first signature and we receive a type mismatch error.
/** @type {Schema & mongoose.Model<Model>} */
const model = mongoose.model('modelName', Schema);

// But something like this would be great.
const model = mongoose.model/**<Model, Schema>*/('modelName', Schema);

My apologies if this is already possible, but I've spend almost a week battling this.

Related: https://github.com/Microsoft/TypeScript-Node-Starter/issues/101

Checklist

My suggestion meets these guidelines:

Stuk commented 5 years ago

I would also like to see something like this. Currently it's really hard to use functions with generic types from JS.

ekulabuhov commented 4 years ago

Another use case with React Hooks:

// useState<string> is derived correctly
const [aString, setAString] = useState("default value");

Removing default value there is no way to pass type info:

// useState<any>
const [aString, setAString] = useState();
pm0u commented 4 years ago

Anything on this? Also stuck on the useState example as shared above:

// Doesn't set type properly
const [error, setError] = /** @type {React.useState<?string>} */ useState(null);
// Nor does this
/** @type {[?string, React.Dispatch<React.SetStateAction<?string>>]} */
const [error, setError] = /** @type {React.useState<?string>} */ useState(null);

// we have an error we want to set now.
setError('Error!') // Shows a type error

Edit: Ok this might be a hack, but in the case of the useState example it works:

const [error, setError] = useState(/** @type {?string} */ (null));

implied types for error are string | null and setError is React.Dispatch<React.SetStateAction<?string>>

i think what's happening "under the hood" is we are forcing the "type" that useState is being passed.

robertknight commented 3 years ago

For functions that take an argument of the generic type, casting the argument itself is reasonably practical. We've settled on these patterns with useState and useRef in React for example:

const [thing, setThing] = useState(/** @type {SomeType|null} */ (null));
const widgetRef = useRef(/** @type {HTMLElement|null} */(null));

In cases where there is no argument for everything else to be inferred from, it gets much more verbose unfortunately. What would be helpful is being able to do something like:

const aSet = /** @type {?<string>} */(new Set());

Where the ? is inferred as Set. In this example it only saves a few characters, but it could be a lot more if the generic type name is lengthy and/or has to be imported from somewhere.

k-yle commented 3 years ago

Where the ? is inferred as Set. In this example it only saves a few characters, but it could be a lot more if the generic type name is lengthy and/or has to be imported from somewhere.

Something like that would be awesome, it would majorly simplify the code required for React.forwardRef and React.memo:


- const Component = /** @type {React.ForwardRefExoticComponent<React.RefAttributes<Props>>} */ (React.forwardRef(() => { ... }))
+ const Component = /** @type {?<Props>} */ (React.forwardRef(() => { ... }))
sebinsua commented 3 years ago

We have the same issue with Recoil.

import {
  atom,
  selector,
  useRecoilState,
  useRecoilValue,
  DefaultValue,
} from "recoil";

/**
 * @typedef {{
 *   a?: string,
 *   b?: string,
 * }} BasicState
 */

// Horrific!
const basicAtom = atom(
  /** @type {import("recoil").AtomOptions<BasicState>} */
  ({
    key: "basicAtom",
    default: {
      a: undefined,
      b: undefined,
    },
  })
);

// Better.
const basicAtom2 = atom({
  key: "basicAtom2",
  /** @type {BasicState} */
  default: {
    a: undefined,
    b: undefined,
  },
});

// Easier to read but perhaps more confusing, because it reveals
// how the defaults properties are seen as mandatory otherwise...
/** @type {import("recoil").RecoilState<BasicState>} */
const basicAtom3 = atom({
  key: "basicAtom3",
  default: {},
});

// This works immediately.
const aSelector = selector({
  key: "aSelector",
  get({ get }) {
    const basicState = get(basicAtom2);

    return basicState.a;
  },
});

// But selectors are horrible!
const bSelector = selector(
  /** @type {import("recoil").ReadWriteSelectorOptions<string>} */
  ({
    key: "bSelector",
    get({ get }) {
      const basicState = get(basicAtom2);

      return basicState.b;
    },
    set({ get, set }, newValue) {
      if (newValue instanceof DefaultValue) {
        return;
      }

      set(basicAtom2, { ...get(basicAtom2), b: newValue });
    },
  })
);

// Is this way of doing selectors preferable? The <any> confuses me...
const bSelector2 = selector({
  key: "bSelector2",
  /**
   * @type {import("recoil").ReadWriteSelectorOptions<any>['get']}
   */
  get({ get }) {
    const basicValue = get(basicAtom2);

    return basicValue.b;
  },
  /**
   * @type {import("recoil").ReadWriteSelectorOptions<string>['set']}
   */
  set({ get, set }, newValue) {
    if (newValue instanceof DefaultValue) {
      return;
    }

    const basicValue = get(basicAtom2);

    set(basicAtom2, {
      ...basicValue,
      b: newValue,
    });
  },
});

// I think this is the best way...
/** @type {import("recoil").RecoilState<string | undefined>} */
const bSelector3 = selector({
  key: "bSelector3",
  get({ get }) {
    const basicValue = get(basicAtom2);

    return basicValue.b;
  },
  set({ get, set }, newValue) {
    if (newValue instanceof DefaultValue) {
      return;
    }

    const basicValue = get(basicAtom2);

    set(basicAtom2, {
      ...basicValue,
      b: newValue,
    });
  },
});

const a = useRecoilValue(aSelector);
const [b, setB] = useRecoilState(bSelector);
const [b2, setB2] = useRecoilState(bSelector2);
const [b3, setB3] = useRecoilState(bSelector3);

if (
  typeof a === "string" &&
  typeof b === "string" &&
  typeof b2 === "string" &&
  typeof b3 === "string"
) {
  setB("str");
  setB((str) => str + "str");

  setB2("str");
  setB2((str) => str + "str");

  setB3("str");
  setB3((str) => str + "str");
}

I don't like having to wrap an argument with brackets in order to be able to apply a comment to it - that seems like I'm needing to alter the implementation, which is something I'd like to avoid.

But it's difficult right now since you have to kind of guess at what to set, in order to keep things simple.

For now, I'll probably do this...

import {
  atom,
  selector,
  DefaultValue
} from "recoil";

/**
 * @typedef {{
 *   a?: string,
 *   b?: string,
 * }} BasicState
 */

const basicStateAtom = atom({
  key: "basicStateAtom",
  /** @type {BasicState} */
  default: {
    a: undefined,
    b: undefined,
  },
});

/** @type {import("recoil").RecoilValueReadOnly<string | undefined>} */
const aSelector = selector({
  key: "aSelector",
  get({ get }) {
    const basicState = get(basicAtom2);

    return basicState.a;
  },
});

/** @type {import("recoil").RecoilState<string | undefined>} */
const bSelector = selector({
  key: "bSelector",
  get({ get }) {
    const basicValue = get(basicStateAtom);

    return basicValue.b;
  },
  set({ get, set }, newValue) {
    if (newValue instanceof DefaultValue) {
      return;
    }

    const basicValue = get(basicStateAtom);

    set(basicStateAtom, {
      ...basicValue,
      b: newValue,
    });
  },
});

For React, it doesn't seem so bad, particularly if you don't mind defining your own UseStateTuple helper type...

import { useState, useRef } from "react";

/**
 * @template S
 * @typedef {[S, import("react").Dispatch<import("react").SetStateAction<S>>]} UseStateTuple
 */

// This is not very nice...
const [aString, setAString] = useState(
  /** @type {string | undefined} */ (undefined)
);

// This is a little long-winded to say the least...
/** @type {[string | undefined, import("react").Dispatch<import("react").SetStateAction<string | undefined>> ]} */
const [aString2, setAString2] = useState();

// This is much, much better...
/** @type {UseStateTuple<string | undefined>} */
const [aString3, setAString3] = useState();

// This is OK...
/** @type {import("react").MutableRefObject<HTMLElement | undefined>} */
const htmlElement = useRef();
trusktr commented 3 years ago

Calling functions with generic types is important in AssemblyScript (TypeScript that compiles to WebAssembly). This would help to possibly get us to a point where we can write working plain JS, but also compile the same code to WebAssembly.

sdegutis commented 3 years ago

Casting the argument sometimes is too verbose. For example, when you pass a map of strings to T, now you have to cast the whole map to /** @type {{ [key: string]: MyInterface }} */. When you want the map values to be () => T or even more complex, the cast gets uglier.

I'd suggest changing the parser to look for a /** @typeparam {MyInterface} */ in between the function expression and the function argument-set parentheses.

petrkrejcik commented 2 years ago

@weswigham Hi, I would like to kindly as if the discussion has evolved? (you've added a tag "In Discussion") Could we hope for this feature?

trusktr commented 2 years ago

I think we should take something into consideration: JSDoc comments are for documentation. We should avoid repurposing JSDoc comments for features that aren't documentation, but call sites for type functions. Otherwise we might make things more difficult for documentation generators that piggy back on JSDoc syntax.

What would the syntax be though? Here's an idea:

// @ts-typeparam T Dog
doSomething(dog)

where the function itself may be documented as:

/**
@template {!Animal} T
@param {T} animal
*/
function doSomething(animal) {...}

This takes advantage of TypeScript's special comment syntax (f.e. // @ts-whatever). Hey look, GitHub syntax highlight even colors the @ts already.

In that example, there is a clear distinction between documentation and usage.

Any other syntax ideas?

phil294 commented 2 years ago

JSDoc comments are for documentation. We should avoid repurposing JSDoc comments for features that aren't documentation

@trusktr JSDoc is has not been only for documentation for a long time now. It supports almost anything TS does, with very few exceptions. As for syntax, the /** */ is well-established and parsed by the compiler if you're programming in JS.


// But something like this would be great.
const model = mongoose.model/**<Model, Schema>*/('modelName', Schema);

There is already existing JSDoc syntax for declaring generic types: @template. So surely this tag should be reused (?)

const model = mongoose.model/** @template Model, Schema */('modelName', Schema);
skapxd commented 2 years ago

I use this way to send types as parameters, but it's a bit dirty

/**
 * @template T
 * @param {Object} props
 * @param {string} props.key
 * @param {T} [props.templateType]
 * @returns {T}
 */
const getLocalStorage = ({ key = "" }) => {
  const data = localStorage.getItem(key);
  return JSON.parse(data);
};

/**
 * @type {{
 * id: number,
 * name: string,
 * phoneNumber: number
 * }}
 */
const templateType = null;
getLocalStorage2({
  templateType,
  key: "somebody",
});
wenq1 commented 2 years ago

is there any plan moving this forward? @RyanCavanaugh

RedGuy12 commented 1 year ago

Any updates?

phaux commented 1 year ago

BTW I just found out this works:

import { useState } from "react"

/** @type {typeof useState<number>} */
const useNumberState = useState

const [value] = useNumberState(123)

or

import { useState } from "react"

const [value] = /** @type {typeof useState<number>} */ (useState)(123)

equivalent to

import { useState } from "react"

const [value] = useState<number>(123)
noinkling commented 1 year ago

For anyone unaware, the above are called "instantiation expressions" (see #47607) and were added in 4.7

Another option is doing something like:

/** @type {ReturnType<typeof fn<SomeType>>} */
const myVar = fn('whatever')

No intermediate variables or nasty inline casting syntax, but I guess less semantically 'pure', and can't be used in certain edge cases where ReturnType doesn't work well.

zhenzhenChange commented 1 year ago

I don't think there's much of a problem with using type assertions (type conversions) :

const elRef = useRef(/** @type {HTMLElement | null} */ (null));
const [value, setValue] = useState(/** @type {SomeType | null} */ (null));

After all, this should essentially be a matter of generic passing. It's just that the readability of the code decreases as the type comment increase. Also note the inline () that wrap the value to form an expression to be valid. Like this: (val) ---> expr


TypeScript Doc

image

Cast Syntax

image

milahu commented 1 year ago

https://github.com/microsoft/TypeScript/issues/27387#issuecomment-659671940

this should be in https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#template

Jemt commented 1 year ago

+1 Any progress on this one ?

thepassle commented 9 months ago

As mentioned in #56102 , I think it'd be really great to add better support for this. Lots of projects are using JSDoc for typechecking instead of full-on TS (either approaches are valid and fine, lets not start that discussion here), and it seems to me like better support for generics in jsdoc is long overdue.

I also think the example proposed by the OP would be a great candidate:

const model = mongoose.model/**<Model, Schema>*/('modelName', Schema);
// or perhaps some variation of an @ tag, that seems more in-line with the current approach JSDoc takes
const model = mongoose.model/** @generic {<Model, Schema>} */('modelName', Schema);

What I think is nice about these approaches is that:

Would love to discuss and see what we can do to get the ball rolling on this, especially considering that this issue has been open for 5 years :)

noinkling commented 9 months ago

@thepassle Technically this is already solved as per https://github.com/microsoft/TypeScript/issues/27387#issuecomment-1223795056 - not really sure why this issue hasn't been closed yet.

For example you can now do:

const model = /** @type {typeof mongoose.model<Model, Schema>} */(mongoose.model)('modelName', Schema);

If you're aware of this but your argument is that you would prefer for there to be a more ergonomic syntax, probably best to say that explicitly.

thepassle commented 9 months ago

Those indeed seem more like workarounds, so I understand the issue is still open.

So yes, I would prefer for there to be a more ergonomic syntax.

mhofman commented 9 months ago

@noinkling, those are not equivalent. A generic method would become unbound from its target if aliased or type annotated that way.

noinkling commented 9 months ago

@noinkling, those are not equivalent. A generic method would become unbound from its target if aliased or type annotated that way.

You mean methods won't have the right this value/type, essentially? I can see the issue with the alias approach in those cases, but nothing new there. Can you give an example where the casting syntax could be problematic/have a different result compared to TS syntax?

In my example both references to the method (runtime and in the annotation) are accessed directly from the object using dot notation (mongoose.model) so I don't see an issue, but maybe I'm missing something.

If you're concerned about the parentheses they don't change anything:

const foo = {
  bar: function() { return this === foo }
};

(foo.bar)();  // true
gustavopch commented 9 months ago
const model = /** @type {typeof mongoose.model<Model, Schema>} */(mongoose.model)('modelName', Schema)

is equivalent to:

const model = (mongoose.model as typeof mongoose.model<Model, Schema>)('modelName', Schema)

not to:

const model = mongoose.model<Model, Schema>('modelName', Schema)

The fact that this ugly but useful workaround exists doesn't mean a proper syntax for actually passing type parameters (instead of casting the whole function) shouldn't be pursued.

noinkling commented 9 months ago

@gustavopch It didn't exist when this issue was created was the point, it was impossible to provide type params unless they corresponded to runtime params. In that sense the person who created the issue might consider it "solved" now. Personally I've learned to take what I can get when it comes to JSDoc, given that it's something of a second-class citizen for TS. But you're right, it would be nice to have a nicer syntax, just wanted to make it clear that there's an option that works now (even if it isn't pretty).

phaux commented 5 months ago

I return to this thread to point out that the "solution" with instantiation expressions doesn't seem to work when the function has multiple overloads:

const fileInput =
  /** @type {typeof document.querySelector<HTMLInputElement>} */
  (document.querySelector)("input[type=file]")

It seems to check all the overloads at once instead of choosing only one:

Type 'HTMLInputElement' does not satisfy the constraint 'keyof HTMLElementDeprecatedTagNameMap'. deno-ts(2344)
Type 'HTMLInputElement' does not satisfy the constraint 'keyof HTMLElementTagNameMap'. deno-ts(2344)
Type 'HTMLInputElement' does not satisfy the constraint 'keyof MathMLElementTagNameMap'. deno-ts(2344)
Type 'HTMLInputElement' does not satisfy the constraint 'keyof SVGElementTagNameMap'. deno-ts(2344)

In TypeScript it works:

const fileInput =
  document.querySelector<HTMLInputElement>("input[type=file]")
noinkling commented 5 months ago

@phaux You're right, worth noting it could potentially be considered a bug: #51694