Skip to content

jsdoc supporting typescript import style #1

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

Merged
merged 2 commits into from
Jul 26, 2019
Merged

Conversation

ricardohbin
Copy link
Owner

@ricardohbin ricardohbin commented Jul 24, 2019

jsdoc uses catharsis to parse the expressions and catharsis.parse does not accept the typescript import expression @param p { import("./a").Pet } - it raises Invalid type expression (because it doesn't accept parentheses) , so this patch does a simple regexp to remove this pattern before the string is parsed.

In this example, @param p { import("./a").Pet } in jsdoc parse time is interpreted like it was @param p { Pet }

Works great if you want are using Typescript to type check your jsdoc - (https://www.typescriptlang.org/docs/handbook/type-checking-javascript-files.html) - compatible with all jsdoc templates

To use it asap, just add this fork in your package.json -> npm install git+ssh://[email protected]:ricardohbin/jsdoc.git#support-ts-jsdocs

`catharsis.parse` does not accept the typescript import expression `@param p { import("./a").Pet }` - it raises `Invalid type expression` . This patch does a simple regexp to remove this pattern before the string is parsed.

So in this example, `@param p { import("./a").Pet }` in jsdoc parse time is interpreted like it was `@param p { Pet }`
@ricardohbin ricardohbin changed the title Supporting typescript import style jsdoc supporting typescript import style Jul 24, 2019
@ricardohbin ricardohbin merged commit f6cb287 into master Jul 26, 2019
@ricardohbin ricardohbin deleted the support-ts-jsdocs branch July 26, 2019 20:50
ricardohbin added a commit that referenced this pull request Jul 26, 2019
* Supporting typescript import style

`catharsis.parse` does not accept the typescript import expression `@param p { import("./a").Pet }` - it raises `Invalid type expression` . This patch does a simple regexp to remove this pattern before the string is parsed.

So in this example, `@param p { import("./a").Pet }` in jsdoc parse time is interpreted like it was `@param p { Pet }`

* Adding README.md
@ricardohbin
Copy link
Owner Author

Publish as npm package too.

More info at https://github.com/ricardohbin/jsdoc readme

@debugmaster
Copy link

debugmaster commented Nov 8, 2019

@ricardohbin may you allow the repo people to create issues?

This logic doesn't work with indexed types.

You can try by yourself:
1.

npm i --save-dev @types/aws-lambda
  1. Add these lines:
/**
 * @typedef {import("aws-lambda").APIGatewayProxyResult["headers"]} Headers
 */
  1. This happens:
ERROR: Unable to parse a tag's type expression for source file <file> in line 1 with tag title "typedef" and text "{import("aws-lambda").APIGatewayProxyResult["headers"]} Headers": Invalid type expression "APIGatewayProxyResult["headers"]": Expected "!", "#", "$", "(", "-", ".", "/", "0", ":", "<", "=", "?", "@", "[]", "\\", "_", "|", "~", "‌", "‍", Unicode combining mark, Unicode decimal number, Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode punctuation connector, Unicode titlecase letter, Unicode uppercase letter, [1-9], or end of input but "[" found.

return tagInfo;
}

// if expression has typescript import types. Ex: @typedef { import("./").Foo }
if (typeExpression.match(/import\(.*\)\./)) {
typeExpression = typeExpression.replace(/import\(.*\)\./, '');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be resolving the type not just replacing the string.
For example this doesn't work even though the index file is exporting a default.

/* @typedef { import('./index') } */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've managed to bodge the problem by adding a regex check

// if expression has typescript inport types which exported by default. Ex @typedef {import("./foo")}
if (typeExpression.match(/import\(.*\)/)) {
        regex = /import\(['"]?(?:.+)\/([^/\.]+)?(?:\..+)?['"]\)/.exec(typeExpression)
        typeExpression = regex[1]
    }

That regex will capture filename of the import. This is very hacky and wont fix the problem if the filename is not the same as the export.
Should I do a pull request?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would really need to parse the file for it to be usable as the default export may not be named exactly the same.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@debugmaster sorry for my delay, I enabled issues only now.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider improving how typeExpression is patched as is done here: https://github.com/polyforest/jsdoc-tsimport-plugin/blob/main/index.js#L134

@shmolf
Copy link

shmolf commented Jun 13, 2020

I'm really looking forward to this patch. Any movement?

@ricardohbin
Copy link
Owner Author

You can use the patched version, I already published to npm

@jakobrosenberg
Copy link

@ricardohbin if I run

node .\node_modules\jsdoc-import-typedef\jsdoc.js config.js

I still get

Invalid type expression "import("
// config.js
/**
 * @typedef { import("@organization/package/path") } SSRConfig
 * /

I'm not sure what I could be doing wrong.

@paulmelero
Copy link

@jakobrosenberg

I don't know if it has something to do but you're not closing the comment block correctly in your code snippet.

Should be

/**
 *
 */

Without a space at the end between * and /.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants