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

Information about function arguments is not parsed from method JSDoc #39

Closed
alexprey opened this issue Nov 29, 2020 · 8 comments
Closed
Assignees
Labels
bug Something isn't working Svelte V3 Issue related to Svelte V3 components
Milestone

Comments

@alexprey
Copy link
Collaborator

Svelte3 parser is not output detailed information about function arguments that stored in JSDoc.

The following example shows the issue:
component.svelte

    /**
     * Computes the answer to your question.
     * @param {string} question a question about life, the universe, everything
     * @returns {number} the answer to all your questions
     */
    export function computeAnswer(question) {
       return question.indexOf("?") >= 0 ? 42 : 23;
    };

output.json

"args": [
{
    "name": "question"
}],

But expected the full information about argument, like this:

"args": [
{
    "name": "question",
    "type": {
        "kind": "type",
        "text": "string",
        "type": "string"
    },
    "description": "question a question about life, the universe, everything"
@alexprey alexprey added bug Something isn't working Svelte V3 Issue related to Svelte V3 components labels Nov 29, 2020
@soft-decay
Copy link
Contributor

I was about to open an issue for this.
I agree with you, and it should also export optional and default. If you have:

/** @param {string} [question=Why?] a question about life, the universe, everything */

then the output would be:

"params": [
    {
        "type": {
            "kind": "type",
            "text": "string",
            "type": "string"
        },
        "name": "question",
        "optional": true,
        "default": "Why?",
        "description": "a question about life, the universe, everything"
    }
],

I propose to override each param from AST parsing by the param with the same name from JSDoc parsing. If there is more @param than AST params, or if their name don't match, should we ignore or include those @param?

This is a breaking change because method item args would be renamed to params to be consistent with the rest of the api (as we discussed in Issue #37).

soft-decay added a commit to soft-decay/sveltedoc-parser that referenced this issue Nov 29, 2020
- Add function getDefaultJSDocType
- Update tests for svelte3/integration/methods
- Fix typings for SvelteMethodItem
- Add example usage
@alexprey
Copy link
Collaborator Author

Optional and default should be alredy done by JSDoc parser.
Lets keep both parameters for now. I think that we can introduce some additional option to the settings to change this behaviour. But for now lets keep it simple

@soft-decay
Copy link
Contributor

soft-decay commented Nov 29, 2020

Ok, seems good. I added some comments in the parseKeywords method about that, but It can be removed if you would want the source code to be more succinct.

Some questions:

  • I was wondering about some of the deprecated stuff in the code. Would it be a good time to remove those if you're planning a v4 release?
  • About the default format of param in parseParamKeyword.
    • name is set to null: check if name exists before adding the param instead?
    • default is set to null: I'm not sure. Maybe not set the key if it does not exist and make it optional, because undefined and null are different?
    • description is set to null: not set the key and make it optional to be consistent with the rest of the api.

@alexprey
Copy link
Collaborator Author

alexprey commented Nov 29, 2020

I was wondering about some of the deprecated stuff in the code. Would it be a good time to remove those if you're planning a v4 release?

Yes, that is a good time to cleanup

@alexprey alexprey added this to the 4.0.0 milestone Nov 29, 2020
@alexprey
Copy link
Collaborator Author

name is set to null: check if name exists before adding the param instead?

When we can't parse name of parameter, so, that is ok to skip all

default is set to null: I'm not sure. Maybe not set the key if it does not exist and make it optional, because undefined and null are different?

Actually, default property valuable and appliable only when optional property for parameter was set to true, instead this should be hidden. null or undefined value for this property means that default value of parameter is null or undefined.

description is set to null: not set the key and make it optional to be consistent with the rest of the api.

Lets keep this optional.

When you done with your PRs I plan to review output API and extend documentation in typings.d.ts file

@soft-decay
Copy link
Contributor

I'm not totally sure about lib/parser.js#L232:

entry.args = entry.value.params.map((param) => param.name);

Should I remove this? It does not have the same format as the other, just an array of string.

@alexprey
Copy link
Collaborator Author

That a svelte2 format, looks like that I'm maintaine correctly the support of function arguments. So, you should update it with new format.

@soft-decay
Copy link
Contributor

Ok thanks. I ended up doing that. It's included in #40.

alexprey added a commit that referenced this issue Nov 30, 2020
fix(v3-parser): Export metadata for method.param items (#39)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Svelte V3 Issue related to Svelte V3 components
Projects
None yet
Development

No branches or pull requests

2 participants