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

Implement a JSDoc @import tag #22160

Closed
DanielRosenwasser opened this issue Feb 24, 2018 · 30 comments · Fixed by #57207
Closed

Implement a JSDoc @import tag #22160

DanielRosenwasser opened this issue Feb 24, 2018 · 30 comments · Fixed by #57207
Labels
Domain: JavaScript The issue relates to JavaScript specifically Domain: JSDoc Relates to JSDoc parsing and type generation Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

Background

#22158 tracks referencing types from a given module using JSDoc-style namepaths. Given that the syntax is somewhat unintuitive and predates the concept of ECMAScript modules, we would like to support a more ergonomic form if we feel it would be helpful.

Options

Bikeshedding time. 🚲 🏠

@from @import

/**
 * @from "express"
 * @import { Request, Response as CoolResponse } from
 * @import Default
 * @import * as ns
 */

Pros

Cons

  • Doesn't totally look like ESModule syntax which is just extra cognitive overhead.

ECMAScript Import-based

/**
 * @import { x, y as z, default as Default } from "express"
 */

Pros

  • Nothing to learn if you already know ECMAScript syntax.

Cons

  • Less optimal for completions

Issues with the Above

The options above don't make it explicit that only types are being imported. We could play around with keyword/tag placement (e.g. @importtype, @import type, etc.)

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus Domain: JSDoc Relates to JSDoc parsing and type generation labels Feb 24, 2018
@aozgaa
Copy link
Contributor

aozgaa commented Feb 26, 2018

This was previously discussed in #14377

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Mar 9, 2018

We're going to hold off on any new JSDoc syntax, and wait for more feedback here. As per #22445, our current recommendation is to wait on #14844 and then use

/**
 * @typedef {import("express")} express
 */

@DanielRosenwasser DanielRosenwasser added the Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature label Mar 9, 2018
@mhegazy mhegazy removed Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: JSDoc Relates to JSDoc parsing and type generation In Discussion Not yet reached consensus labels Mar 9, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Mar 9, 2018

closing in favor of #14844

@mhegazy mhegazy closed this as completed Mar 9, 2018
@jkrems
Copy link

jkrems commented May 7, 2018

@DanielRosenwasser Did the syntax for module namespaces change? I tried scanning through the linked issues but since none of them are showing the JSDoc equivalent, it's somewhat hard to follow.

/**
 * The following works using latest typescript@next (2.9.0-dev.20180506):
 *
 * @typedef {import('http').IncomingMessage} IncomingMessage
 * @typedef {import('http').ServerResponse} ServerResponse
 *
 * But this fails:
 *
 * > Module 'http' does not refer to a type, but is used as a type here.
 *
 * @typedef {import('http')} http
 */

@mhegazy
Copy link
Contributor

mhegazy commented May 7, 2018

Use typeof:

/** @typedef {typeof import('http')} http*/

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
@DanielRosenwasser DanielRosenwasser added In Discussion Not yet reached consensus Salsa Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: JSDoc Relates to JSDoc parsing and type generation labels Aug 10, 2018
@microsoft microsoft unlocked this conversation Aug 10, 2018
@nojvek
Copy link
Contributor

nojvek commented Aug 14, 2018

Hi Typescript Magic Elves,

Do you think you'll be able to get any decisions on what the final DRY-er jsdoc import syntax will look like?

Just curious what the blocking items are?

@DanielRosenwasser
Copy link
Member Author

The original motivation to not do this was to just re-use as much well-understood JSDoc as we could. If we can get more feedback, I think it will be a strong enough signal that there's interest and that we should actually invest in the feature.

@nojvek
Copy link
Contributor

nojvek commented Aug 15, 2018

I hope this doesn’t get stuck in analysis paralysis fever like some of the other issues.

I am Typescript js-doccing a medium sized project and this is a pretty painful experience. It feels like a very subpar experience to TS.

@nojvek
Copy link
Contributor

nojvek commented Sep 19, 2018

Kind ping to typescript authors and @DanielRosenwasser . Has any decision been regarding this?

Using tsc as a typechecker for js with a verbose syntax like this adds up quite a bit of friction.

@weswigham weswigham added Domain: JavaScript The issue relates to JavaScript specifically and removed Domain: JavaScript The issue relates to JavaScript specifically labels Nov 29, 2018
@nojvek
Copy link
Contributor

nojvek commented Mar 4, 2020

It's been more than 2 years. @DanielRosenwasser is there any progress on this ?

@kfdf
Copy link

kfdf commented Mar 12, 2020

I'm new to javascript/typescript and might be misunderstanding something, but doesn't this work?

// lib.js
/**
@typedef {object} Person
@property {string} name 
@property {number} age */

/**
@template T
@typedef {object} Node 
@property {T} value
@property {Node<T>} next */

/** 
@type {Person} */
export const Person = null;

/** 
@template T
@type {Node<T>} */
export const Node = null;

And then

import { Person, Node } from './lib.js';

/**
@param {Node<Person>} head */
function printList(head) {
  if (!head) return;
  let { name, age } = head.value;
  console.log(name, age);
  printList(head.next);
}
printList({ 
  value: { age: 5, name: 'John' },
  next: {
    value: { age: 10, name: 'Ann' },
    next: null
  }
});

And I got all the red squiggles and intellisence I needed when typing that.
Sure it does spill some bits of typescript declarations into your javascript but other than that it seems like relatively smooth experience...

@ExE-Boss
Copy link
Contributor

@kfdf It’s ugly and may cause unwanted circular dependencies, which can cause breakages at runtime because of unexpected module execution order.

@rjgotten
Copy link

rjgotten commented Mar 13, 2020

@kfdf
but doesn't this work?

Completely different problem-space.

There are TypeScript (and VS Code JS intellisense) constraints you cannot express with JSDoc @typedef tags and thus need .d.ts files. And it's importing 'definitions only' from those .d.ts files into JS files which is the problem.

@bmeck
Copy link

bmeck commented Apr 16, 2020

Just want to note the workaround in #22160 (comment) seems to drop the documentation of the type:

/**
 * This is documented
 * @typedef {{foo:'bar'}} Foo
 */
/**
 * @typedef {import('foo').Foo} Foo
 */
// Foo does not have a description

@ExE-Boss
Copy link
Contributor

Also, #22160 (comment) doesn’t support optional type parameters.

@jeffersoneagley
Copy link

Any updates on this? I'm still importing things at the top of the file, but CRA of course complains in the preflight.

@noinkling
Copy link

noinkling commented Jul 29, 2020

I've resorted to adding export as namespace foo to my .d.ts modules. It makes the namespace globally available which isn't ideal, but it's the only way I can find (assuming we don't want to import actual code) to avoid repeated import(...) spam, since there doesn't seem to be any way to alias a namespace within JSDoc that works (which I've previously commented on in another issue: #14233 (comment)).

@RobinBlomberg
Copy link

@jeffersoneagley I had great success with approach #1, and I found a way to make it interoperable with normal .js/.ts files inside ESM modules. This approach takes advantage of how TypeScript appears to merge file extensions if the filenames are the same.

my-module.ts

export type FooString = 'foo' | 'bar' | 'baz';

export type FooObject = { foo: FooString; };

my-module.js

/**
 * @file Dummy export in order to avoid a runtime ERR_MODULE_NOT_FOUND error.
 */

export {};

index.js

import * as MyModule from './my-module.js';

/**
 * @param {MyModule.FooString} fooString
 * @return {MyModule.FooObject}
 */
export const createFooObject = (fooString) => {
  return { foo: fooString };
};

Now it seems a bit silly to have to create a dummy export file every time, but in my case, I need to define and import 100+ namespaced types, and I really haven't found any other reasonable way to do it.

@nanxiaobei
Copy link

Hope to use JSDoc syntax to import typescript definition to *.js file.

@jeffersoneagley
Copy link

@RobinBlomberg I actually define mine in types.d.ts files around the app now and they basically give every js file in the directory the ability to access types defined in pure TS (don't use any import statements other than inline though, top-of-file imports will turn the type file into a module and then you can't easily import it into js)

@RobinBlomberg
Copy link

@jeffersoneagley Genius! I'll have to try this.

bh213 pushed a commit to bh213/pdf.js that referenced this issue Jun 3, 2022
The Viewer API definitions do not compile because of missing imports and
anonymous objects are typed as `Object`. These issues were not caught
during CI because the test project was not compiling anything from the
Viewer API.

As an example of the first problem:

```
/**
 * @implements MyInterface
 */
export class MyClass {
    ...
}
```

will generate a broken definition that doesn’t import MyInterface:

```
/**
 * @implements MyInterface
 */
export class MyClass implements MyInterface {
    ...
}
```

This can be fixed by adding a typedef jsdoc to specify the import:

```
/** @typedef {import("./otherFile").MyInterface} MyInterface */
```

See jsdoc/jsdoc#1537 and
microsoft/TypeScript#22160 for more details.

As an example of the second problem:

```
/**
 * Gets the size of the specified page, converted from PDF units to inches.
 * @param {Object} An Object containing the properties: {Array} `view`,
 *   {number} `userUnit`, and {number} `rotate`.
 */
function getPageSizeInches({ view, userUnit, rotate }) {
    ...
}
```

generates the broken definition:

```
function getPageSizeInches({ view, userUnit, rotate }: Object) {
    ...
}
```

The jsdoc should specify the type of each nested property:

```
/**
 * Gets the size of the specified page, converted from PDF units to inches.
 * @param {Object} options An object containing the properties: {Array} `view`,
 *   {number} `userUnit`, and {number} `rotate`.
 * @param {number[]} options.view
 * @param {number} options.userUnit
 * @param {number} options.rotate
 */
```
@NemoStein
Copy link

NemoStein commented Jul 11, 2022

It seems that this issue will never reach a proper conclusion, but this is blocking #46011.
We need a way of opting out of @typedef auto exporting the type!

/** @import { Type } from './path/to/module.js' */ would be, IMO, the perfect solution, but something else is needed if this isn't possible.

rousek pushed a commit to signosoft/pdf.js that referenced this issue Aug 10, 2022
The Viewer API definitions do not compile because of missing imports and
anonymous objects are typed as `Object`. These issues were not caught
during CI because the test project was not compiling anything from the
Viewer API.

As an example of the first problem:

```
/**
 * @implements MyInterface
 */
export class MyClass {
    ...
}
```

will generate a broken definition that doesn’t import MyInterface:

```
/**
 * @implements MyInterface
 */
export class MyClass implements MyInterface {
    ...
}
```

This can be fixed by adding a typedef jsdoc to specify the import:

```
/** @typedef {import("./otherFile").MyInterface} MyInterface */
```

See jsdoc/jsdoc#1537 and
microsoft/TypeScript#22160 for more details.

As an example of the second problem:

```
/**
 * Gets the size of the specified page, converted from PDF units to inches.
 * @param {Object} An Object containing the properties: {Array} `view`,
 *   {number} `userUnit`, and {number} `rotate`.
 */
function getPageSizeInches({ view, userUnit, rotate }) {
    ...
}
```

generates the broken definition:

```
function getPageSizeInches({ view, userUnit, rotate }: Object) {
    ...
}
```

The jsdoc should specify the type of each nested property:

```
/**
 * Gets the size of the specified page, converted from PDF units to inches.
 * @param {Object} options An object containing the properties: {Array} `view`,
 *   {number} `userUnit`, and {number} `rotate`.
 * @param {number[]} options.view
 * @param {number} options.userUnit
 * @param {number} options.rotate
 */
```
@jespertheend
Copy link
Contributor

These are snippets of some of my real world code 🥲
image
image
image

What can I say, it is what it is. I still prefer this rather than having to deal with a build step and import maps though.

@DanielRosenwasser DanielRosenwasser removed the Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature label Mar 26, 2024
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 5.5.0 milestone Mar 26, 2024
@DanielRosenwasser
Copy link
Member Author

Thanks to @a-tarasyuk, this should be in the next nightly release and in TypeScript 5.5. The syntax is based on ECMAScript imports:

/**
 * @import * as foo from "some-module-path"
 */

/**
 * @import { x, y as z, default as Default } from "another-module-path"
 */

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: JavaScript The issue relates to JavaScript specifically Domain: JSDoc Relates to JSDoc parsing and type generation Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.