Skip to content

Extending @typedefs by adding additional properties doesn't seem to work? #42877

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

Open
sami616 opened this issue Feb 19, 2021 · 6 comments
Open
Assignees
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Milestone

Comments

@sami616
Copy link

sami616 commented Feb 19, 2021

I know this can be done with an intersection type, but that can often get quite verbose as it often requires an additional typedef to be defined.

As you can see in the below screenshot, i would expect propThree to be present in the extended const.

Is this syntax supported by vscode, looking around it seems to be in webstorm?

Screenshot 2021-02-19 at 10 20 36

Sandbox example: https://codesandbox.io/s/vigilant-https-7h845?file=/src/Animal.js

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 23, 2021
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.3.0 milestone Feb 23, 2021
@sandersn sandersn added Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Feb 24, 2021
@sandersn
Copy link
Member

@sami616 That's not a feature I'm aware of in jsdoc.app or closure, which are the two main systems that typescript tries to maintain compatibility with.

I did a quick search for usage in the code bases we run tests on and could only find 5 uses total, in discord.js and cassandra-driver.
In addition, react-native uses it in a different way: @typedef {Array} followed by @property to indicate an array of option types.

To move forward with this feature, we'd need:

  • Evidence of lots of existing usage, so that existing code would start working better in VS Code.
  • OR An explanation of why this is better than existing solutions, like intersections or keeping types in d.ts files, so that people could start writing this in JS files.
  • A proposal for how it should work.
  • A sketch of how the implementation would fit into the existing @typedef code.
  • An explanation of what to do about alternative syntaxes.

That's a lot of work, so my feeling is that this feature isn't worth it.

@ITenthusiasm
Copy link

@sandersn I wouldn't expect the feature to work as described in the OP. However, I would hope that something like this would work:

/**
 * @typedef {Object} ParentType
 * @property {number} prop I want to keep my JSDocs
 */

/**
 * @typedef {Object} ChildType
 * @extends {ParentType}
 * @property {string} field Here is the type information for the child
 */

This gives me an ergonomic way to extend the base type while maintaining any JSDocs associated with that type. If it's unfitting to use @extends or @implements for this, perhaps it could be worthwhile to introduce a custom tag? As mentioned in #49905, the problem with sticking to intersection is that we end up leaking internal, intermediate types with the intersection approach:

/**
 * @typedef {Object} ParentType
 * @property {number} prop I want to keep my JSDocs
 */

/**
 * I get leaked
 * @typedef {Object} BaseChildType
 * @property {string} field Here is the type information for the child
 */

/** @typedef {ParentType & BaseChildType} ChildType */

This could be a source of confusion, and/or it could result in the over-suggestion of irrelevant types in IDEs.

@ITenthusiasm
Copy link

Responding to some of the earlier concerns:

Evidence of lots of existing usage

This approach probably isn't being used right now because it doesn't work. But pretty much any existing codebase that defines an interface and uses interface Child extends Parent would arguably be evidence for this use case. It's just that there aren't as many JSDoc users yet; but the use case is the same. With some of Rich Harris's philosophies on JSDocs getting more popularized, we may see this more "explicitly" in libraries in the future.

Why Not .d.ts Files?

This approach isn't as convenient when it comes to library development. In normal TS land, it makes sense to export related types and functions from the same .ts file. So naturally, it would make sense to do the same thing for a .js file. Unfortunately, we cannot; and there are caveats that come from using a separate .d.ts file.

For end users, it's inconvenient to write

import { myFunction } from "my-library";
import type { myTypeRelatedToFunction } from "my-library/some-types-file";

for every JS file that has complex types. (Barrel files are not a silver bullet here because not all consumers have access to tree shaking.)

At the same time, for library maintainers, it's inconvenient to write

/library
  my-js-file.js
  my-js-file-types.d.ts

every time complex types come into play. But beyond the minor file-structure inconvenience, it makes it makes the build process more difficult. Where do the types go? If some types are in the .d.ts file while the actual implementations (functions, classes, etc.) are in the js file, then there's a split in where the related types exist. Do we create a new .d.ts file that exports types from both files? How would maintainers automate that process and make it reliable? (Does that dilemma make sense?) Developers using .ts files never have to answer these questions (if they don't want to).

@sandersn
Copy link
Member

But pretty much any existing codebase that defines an interface and uses interface Child extends Parent would arguably be evidence for this use case.

I think you're better off specifying an @interface tag since @typedef is the equivalent of Typescript type aliases, which do not support inheritance. @interface is tracked by #41675

Of course on that issue you'll need to discuss "Why not .d.ts files". Feel free to ping me on that; my quick opinion is that dividing import/import type is a good thing. But I'd like to understand the build problems for library maintainers better.

@ITenthusiasm
Copy link

Thanks for referencing that issue! I'll add some comments tomorrow -- Lord wiling.

@ITenthusiasm
Copy link

Added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants