Skip to content
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

Mapped keys do not preserve JSDoc documentation #50715

Open
devmanbr opened this issue Sep 11, 2022 · 9 comments
Open

Mapped keys do not preserve JSDoc documentation #50715

devmanbr opened this issue Sep 11, 2022 · 9 comments
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@devmanbr
Copy link

Bug Report

πŸ”Ž Search Terms

mapping types with docs; preserve docs on mapped types

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about this

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

All relevant code can be found in the example links.

Why doesn't TypeScript/Vscode preserve documentation / JSDoc for mapped types?

Using this example that appears in the TypeScript documentation itself, which you can quickly reproduce in Vscode locally, you can see the problem.

It is expected that when remapping the keys of a type object, the documentation would remain what was defined in the original/previous keys, but this does not happen. Nothing is preserved.

This sucks in the development context, because it requires us to have to repeat the documentation of a property or method, for example, in multiple places, thousands of times. This can lead to consistency issues as the code grows and loses the point of automating things.

Another directly related problem is in Index Signatures. Even if documentation is added, nothing is preserved. This also includes Union Types. An example can be seen here.

πŸ™ Actual behavior

JSDoc documentation is not preserved in mapped keys.

πŸ™‚ Expected behavior

Mapped keys must have JSDoc documentation of their original/previous names.

@jakebailey
Copy link
Member

This seems like an extension of #47933; if you write code like (Playground Link):

type someType = {
  /**
   * some text to be displayed
   */
  [key:string]: string;
};

declare const test: someType;

someType.anything;

Hovering over anything gives you back the docstring as expected, but it doesn't look like the documentation follows through to use of that declaration in a contextual context where you're assigning to someType.

@jakebailey jakebailey added Suggestion An idea for TypeScript Help Wanted You can do this Experience Enhancement Noncontroversial enhancements labels Sep 12, 2022
@jakebailey jakebailey added this to the Backlog milestone Sep 12, 2022
@jakebailey
Copy link
Member

FYI @danay1999

@danay1999
Copy link
Contributor

@jakebailey Thank you for the heads up. It seems the main issue is that it doesn't conserve the JSDoc from the original keys, and therefore it has nothing to show. I am not sure if it would count as an extension of #47933

@devmanbr
Copy link
Author

What I cited was just a brief example. But I haven't come across types working as they should (as I reported) in real use cases.

@devmanbr
Copy link
Author

I know it's hard to project this, but as far as the primary problem at least is there any light on when it might be resolved?

@Peeja
Copy link
Contributor

Peeja commented Mar 31, 2023

I'd like to have this, too. Having dug into the code, it looks like this is mostly an intentional design decision. I'm interested in revisiting that, if it is. That said, I think there are a few potential issues with just blithely passing along the documentation.

The most significant one is simply that the documentation may well not be correct for the mapped properties. Consider a version of your example above using setters:

type Setters<Type> = {
  [Property in keyof Type as `set${Capitalize<string & Property>}`]: (value: Type[Property]) => void
};

The documentation "Person's name." doesn't really fit the method setName(). I mean, you can probably follow along in this case, but you can see how slightly more exotic cases would end up with some really wacky and confusing documentation strings.

But since there are some solid use cases for being able to bring the documentation along, I wonder if it might be worthwhile to add some kind of template-literal-like syntax to the JSDoc to enable it explicitly. Consider something like

type Setters<Type> = {
  /** Set ${Uncapitalize<docof Property>} */
  [Property in keyof Type as `set${Capitalize<string & Property>}`]: (
    /** New value for ${Uncapitalize<docof Property>} */
    value: Type[Property]
  ) => void
};

interface Person {
  /** Person's name */
  name: string;
  /** Person's age */
  age: number;
}

const me: Setters<Person> = {
  // πŸ’¬ Documentation: "Set person's name"
  setName: (value) => {},
  // πŸ’¬ Documentation: "Set person's age"
  setAge: (value) => {}
};

// πŸ’¬ Documentation on parameter: "New value for person's name"
me.setName("Jake Carter")
// πŸ’¬ Documentation on parameter: "New value for person's age"
me.setAge(35)

The other way to go would be the proposal for full on imperative types. I'm not sure if this idea is stronger or weaker for being more constrained than that.

@devmanbr
Copy link
Author

your observations are very valid, @Peeja. I honestly don't know what the best way is, but I really hope that something can be done to allow for this more "dynamic" control at the documentation level on mapped types.

@danvk
Copy link
Contributor

danvk commented Aug 29, 2023

I was that Pick preserves JSDoc but some simple variations do not:

interface Person {
  /** Person's name */
  name: string;
  /** Person's age */
  age: number;
}

type PickName = Pick<Person, 'name'>;
const pickName: PickName = {
  name: 'alice',  // <-- has JSDoc
};

interface ExplicitName {
  name: Person['name'];
}
const explicitName: ExplicitName = {
  name: 'bob',  // <-- no JSDoc
};

JSDoc seems to get preserved by a "homomorphic mapped type" but not by anything else.

danielbankhead added a commit to googleapis/google-auth-library-nodejs that referenced this issue Oct 25, 2023
danielbankhead added a commit to googleapis/google-auth-library-nodejs that referenced this issue Oct 26, 2023
* feat: Expose `Gaxios` instance and default

* feat: Unify Base `AuthClient` Options

* docs: clean up documentation

* chore: Discourage external use via `@internal`

* refactor: minor

* refactor: clean up `transporter` interface, options, and docs

* Merge branch 'main' of github.com:googleapis/google-auth-library-nodejs into authclient-enhancements

* Merge branch 'main' of github.com:googleapis/google-auth-library-nodejs into authclient-enhancements

* test: Init tests for `AuthClient`

* fix: Use entire `JWT` to create accurate `createScoped`

* chore: update `typescript` in fixtures

* test: Use Sinon Fake Timer to Avoid Flaky Time Issue (#1667)

* test: Use Sinon Fake Timer to Avoid Flaky Time Issue

* chore: change order for clarity

* docs(sample): Improve `keepAlive` sample with `transporterOptions`

* docs: Docs-deprecate `additionalOptions`

* refactor: un-alias `getOriginalOrCamel`

* chore: remove confusing duplicate interface

* docs: nit

* docs: Improve camelCased option documentation

We can refactor once microsoft/TypeScript#50715 has been resolved.

* refactor: Unify `OriginalAndCamel` & `SnakeToCamelObject`

+ make recursive

* docs: Add issue tracking link

* refactor: Replace `getOriginalOrCamel` with a cleaner API

* feat: Allow optional `obj`

* refactor: re-add `SnakeToCamelObject`

* refactor: dedupe interface for now

* feat: Add camelCase options for `IdentityPoolClient`
Offroaders123 added a commit to Offroaders123/Dino-TSX that referenced this issue Nov 22, 2023
Realized it would make sense if my automatic JSX types could have their JSDoc annotation descriptions from the base `HTMLElement`-kind of interface definitions they are already based on, but that doesn't appear to be possible for mapped types unfortunately.
https://jsdoc.app/tags-borrows
https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#link (this is an alternative, where `@borrows` won't work [i'm not sure if it works in TS anyways, but yeah], `@link` is a nice alternative if that eventually does work :) )
https://stackoverflow.com/questions/70766995/how-can-i-link-something-that-isnt-imported (not related specifically, but neat otherwise :D )
microsoft/TypeScript#50715

The next thing I thought of is that an option to get automatic types for JSX element definitions based on `HTMLElement`-based interfaces more narrowed than it currently is. Right now it lets you assign attributes that are readonly, and also assign to element methods, which isn't ideal. It could just be 'ignored', but this defeats the purpose of TypeScript, which should prevent accidentally overriding that kind of thing. Most JSX types implementations declare their own types for the element creation, I think that's a big extra though, in terms at least with what and why I'm trying to work on this library. Since it's creating standard elements, I think it makes a lot of sense to derive the JSX types from those dynamically, hence that will also allow you to get automatic type definitions for Web Component classes, and it will work without the implementor of that component to provide their own JSX types for this project. The key is that it works in your favor, you don't need to make it work with this tool. (Inspired behind the TypeScript origin story; https://www.youtube.com/watch?v=U6s2pdxebSo&t=2200s)

Since the types for the `HTMLElement` prototype tree are essentially a static list of members (the only difference being new features added down the line), it seems feasible to omit specific keys which shouldn't be assignable/visible in JSX, simply by removing keys that are present in parent prototypes of the base `HTMLElement` class. I went up the prototype tree to get `HTMLElement > Element > Node > EventTarget > Object`. Now I can go through these values to deduce which ones can be removed from the JSX types, hence allowing only the necessary ones to be accessible from the user's point of view when assigning props and such.
https://www.typescriptlang.org/docs/handbook/utility-types.html#omittype-keys
@SushantChandla
Copy link

Adding to this,

type Text= {
  otherProperty: string,
  /**
    * @deprecated <- This is also not working
  */
  [key: string]: unknown;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants