-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow to explicitly pass type parameters via JSDoc #27387
Comments
I would also like to see something like this. Currently it's really hard to use functions with generic types from JS. |
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(); |
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 i think what's happening "under the hood" is we are forcing the "type" that useState is being passed. |
For functions that take an argument of the generic type, casting the argument itself is reasonably practical. We've settled on these patterns with 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 |
Something like that would be awesome, it would majorly simplify the code required for - const Component = /** @type {React.ForwardRefExoticComponent<React.RefAttributes<Props>>} */ (React.forwardRef(() => { ... }))
+ const Component = /** @type {?<Props>} */ (React.forwardRef(() => { ... })) |
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 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(); |
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. |
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 I'd suggest changing the parser to look for a |
@weswigham Hi, I would like to kindly as if the discussion has evolved? (you've added a tag "In Discussion") |
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. In that example, there is a clear distinction between documentation and usage. Any other syntax ideas? |
@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
There is already existing JSDoc syntax for declaring generic types: const model = mongoose.model/** @template Model, Schema */('modelName', Schema); |
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",
});
|
is there any plan moving this forward? @RyanCavanaugh |
Any updates? |
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) |
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 |
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. |
+1 |
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 :) |
@thepassle Technically this is already solved as per #27387 (comment) - 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. |
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. |
@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 In my example both references to the method (runtime and in the annotation) are accessed directly from the object using dot notation ( If you're concerned about the parentheses they don't change anything: const foo = {
bar: function() { return this === foo }
};
(foo.bar)(); // true |
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. |
@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). |
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:
In TypeScript it works: const fileInput =
document.querySelector<HTMLInputElement>("input[type=file]") |
@sandersn I am looking into implementing something like:
Do you think there's a chance this approach will be accepted? |
Our approach to JSDoc has evolved since this issue was created. At that time I didn't want to add anything that didn't have precedent in the jsdoc documentation generator, or at least in Closure compiler. However, Typescript is the largest processor of JSDoc at this point, and TS users who want typing are probably the most prolific authors of it. So we've decided on a few new principles:
(1) means that TS users will be able to use the new JS syntax with a minimum of surprise. (2) means that jsdoc processors (including TS itself!) will be able to understand the new tag with a minimum of new parsing. Re (2c), the rough descending order of goodness, or familiarity, in my opinion is:
I think we can come up with something for (3). This is a very long way to say that, yes, we'd consider a tag for this that looks a lot like your @DanielRosenwasser you've thought about jsdoc a lot too and you may have interesting insights from working on the types-in-JS proposal, which ran into exactly this problem. |
I've got a PR ready for the following syntax: let a = /** @specialize <number> */(f());
/** @specialize <string, number> */
let b = f();
/** @specialize {string} */
let c = f`template with ${variable}`;
let d = /** @specialize <string> */(new D()); So basically one can put the This will also work for JSX, though the element with type arguments will need to put inside curly braces so that we can also add JSDoc there: function MyForm() {
return (
<form>
{/** @specialize {number} */(
<Input value={0} />
)}
</form>
)
} |
It's something we'll have to consider. It's not perfect, but the solution right now is to cast to a value that's already specialized: /**
* @template T
* @param x {T}
*/
function f(x) {
return x;
}
let obj = {
/**
* @template T
* @param x {T}
*/
f(x) {
return x;
}
};
let g = /** @type {typeof f<string>} */ (f);
// ^?
// typeof f<string>
let objG = (/** @type {typeof obj.f<string>} */ (obj.f)).bind(obj);
// ^?
// typeof obj.f<string> Though noted above in #27387 (comment), it doesn't work in the case of overloads. |
Good point! Btw, the reason this currently does not work: const fileInput =
/** @type {typeof document.querySelector<HTMLInputElement>} */
(document.querySelector)("input[type=file]") is already pointed out in the other issue, here: Do you think there's a chance the other PR will be merged? i.e. #51695 |
I'm a big TypeScript fan that's trying out a new project where I use JSDocs instead of TS. Here are some of the learnings I've been forced to do along the way. Passing in a generic slot to a functionThe key here is utilizing TypeScript version: const patchReq = async <T>(path: string, data: Partial<T>): T => makeRequest('PATCH', path, data);
const updateBallot = async (ballot: Partial<Ballot>) => patchReq<Ballot>(`elections/${ballot.id}`, ballot);
// ^? returns Promise<Ballot> JSDocs version: /** @template T; @param {string} path; @param {Partial<T>} data; @returns {Promise<T>} */
const patchReq = async (path, data) => makeRequest('PATCH', path, data);
/** @param {Partial<Ballot>} ballot; @returns {ReturnType<typeof patchReq<Ballot>>} */
const updateBallot = async (ballot) => patchReq(`ballots/${ballot.id}`, ballot); Warning As pointed out by @phaux above, this often fails when there are overloads. React useStateThis was already mentioned above, but I think it's useful to rewrite. Typecasting the initial value has never failed me. Make sure to remeber the parentheses const [optionalTitle, setOptionalTitle] = useState(/** @type {null | string} */(null));
// ^? null | string Note I would prefer to have instead written on the line before the useState statement |
* Generate `composeMiddleware` overloads up to 30 arguments. * Use file extensions in `import`s. * Use `Simplify` in the return type of `composeMiddleware` for better ergonomics. * Add `extends {}` to the type parameters of a `Middleware` function--I don't actually remember why, but the types didn't infer correctly otherwise. * Use `const`s instead of `function`s for middleware functions. The `@type` tag doesn't apply correctly to a `function` declaration--which makes sense, since there's no equivalent syntax that would do it in TypeScript: microsoft/TypeScript#27387
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.My apologies if this is already possible, but I've spend almost a week battling this.
Related: microsoft/TypeScript-Node-Starter#101
Checklist
My suggestion meets these guidelines:
The text was updated successfully, but these errors were encountered: