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

Support unexported module functions #37

Closed
whitlockjc opened this issue Aug 5, 2018 · 17 comments
Closed

Support unexported module functions #37

whitlockjc opened this issue Aug 5, 2018 · 17 comments

Comments

@whitlockjc
Copy link

whitlockjc commented Aug 5, 2018

Let's say I've got the following JSDoc to describe a callback:

/**
 * Callback used to provide access to altering a remote request prior to the request being made.
 *
 * @callback PrepareRequestCallback
 *
 * @param {object} req - The Superagent request object
 * @param {string} location - The location being retrieved
 * @param {function} callback - First callback
 */

Right now, I have an NPM module that wants to describe a callback but it's not something exported. If I use @callback, I get the following error when generating the TSD:

Can't add member 'module:path-loader~PrepareRequestCallback' to parent item 'undefined'. Unsupported member type: 'alias'

If I replace @callback with @function, no error but the function is exported and I end up with something like this:

/**
 * Utility that provides a single API for loading the content of a path/URL.
 */
declare module 'path-loader' {
    // ...

    /**
     * Callback used to provide access to altering a remote request prior to the request being made.
     * @param req - The Superagent request object
     * @param location - The location being retrieved
     * @param callback - First callback
     */
    export function PrepareRequestCallback(req: object, location: string, callback: Function): void;

    // ...
}

I need a way within a @module to describe a callback so that it gets generated as declare function {MY_CALLBACK} so that it's documented but not exported. (I'm new to TypeScript and TSD so bear with me please.)

@whitlockjc
Copy link
Author

And I could be doing all of this wrong. I just want someone to know what the expectation is when writing a callback is. But it's not an exported function.

@whitlockjc
Copy link
Author

whitlockjc commented Aug 5, 2018

The issue about alias being an unsupported kind seems to happen once I try to throw things into a module. Without module, @typedef {function} MyCallback works just fine and is declared as declare type MyCallback. But once I try to define MyCallback as an internal type of a module, that error happens.

So I can't tell if there are multiple things going on but any help would be appreciated. If you want some context, I'm trying to do this for path-loader and eventually for others once I get the simplest project done.

@whitlockjc whitlockjc changed the title Support @callback Support unexported module functions Aug 5, 2018
@whitlockjc
Copy link
Author

I changed the description of this because it seems like @callback works fine when not using @module as well. I've updated path-loader to have the desired @callback support, with generated TSD and updated the repo. What I want is to be able to use @module so that LoadOptions, PrepareRequestCallback and ProcessResponseCallback are declare type within the module and only load is exported.

@whitlockjc
Copy link
Author

I just got it somewhat working by marking PrepareRequestCallback and ProcessResponseCallback as @global but why I don't have to do that for LoadOptions is beyond me. It would be slick if they all were treated as non-global but I can't seem to get @callback to work with @module at this time.

@whitlockjc
Copy link
Author

I've also tried @callback module:path-loader~PrepareRequestCallback and I get a similar error:

Can't add member 'module:path-loader~PrepareRequestCallback' to parent item 'undefined'. Unsupported member type: 'alias'

I'll start debugging code.

@tineheller
Copy link
Collaborator

I'm not sure, maybe there are more problems.

First about the case that you just want an unexported function inside a module, like that:

declare module myModule {
    function myFunction(...): void;
}

I think the correct way would be, to use the @inner tag for myFunction (http://usejsdoc.org/tags-inner.html).

Unfortunately this will not work for now because there is a bug. (At the moment, you will get an unexported function, if you use @private. But then you have to call jsdoc with the private flag...).

I think we can fix this bug next week or so.

@whitlockjc
Copy link
Author

Is that the right way to do this? I'm new to TypeScript in general. I've looked into some documentation and I can't tell if declare type or declare function is the right thing. All I want to do is document a callback that isn't exported (it shouldn't be) and isn't an instantiable thing. I just want to document it for tooling purposes.

@tineheller
Copy link
Collaborator

Actually a callback is a typedef. So for example if you have this documentation:

 /**
 * @callback testCallback
 * @param {string} testCBParam
 */
 /**
 * @function myFunction
 * @param {string} myString
 * @param {testCallback} myCallback
 */

you will get this output:

/**
 * @param testCBParam
 */
declare type testCallback = (testCBParam: string)=>void;
/**
 * @param myString
 * @param myCallback
 */
declare function myFunction(myString: string, myCallback: testCallback): void;

The problem is, if you add the callback to a module. I just reproduced that. You can add the function and the callback to a namespace or you can add only the function to a module. That works fine. But if you add the callback to a module you will get the error message that you mentioned above:

Can't add member 'module:myTestModule~testCallback' to parent item 'undefined'. Unsupported member type: 'alias'

This seems to be another bug! and I think we also can fix this during next week.

@whitlockjc
Copy link
Author

That's correct. :)

@whitlockjc
Copy link
Author

I will gladly help with a PR, this is blocking me. Let me see if I can figure out where this is failing and go from there.

@whitlockjc
Copy link
Author

whitlockjc commented Aug 7, 2018

It looks like in the module handling, the kind is alias and that is just not converted to the real kind it's an alias of. I have a little code that does this but what ends up happening is that the generated code is export function myFunction(myString: string, myCallback: testCallback): void; instead of declare function myFunction(myString: string, myCallback: testCallback): void;. So progress is made but I need to figure out how declare vs. export is handled. Once I do that, I should have a PR. I think the alias handling is where the magic is but not sure what all values for type.kind are when kind is alias, I'm not sure.

@whitlockjc
Copy link
Author

whitlockjc commented Aug 7, 2018

It looks like the JSDoc name and scope are not being used to identify whether or not something should be declared or exported. When I look at the JSDoc details for my file, it's clear that module.exports.load should be exported by its name (starts with exports. or module.exports.) and by its scope (static) whereas my callback/typedefs, which are defined for documentation/typing information alone, should not be exported based on their name (does not start with exports. or module.exports.) and their scope (inner) .

Any reason why these details are not used when identifying if/when things should be declared vs. exported?

@whitlockjc
Copy link
Author

I stand corrected, you are using .scope but I'm not sure how inner ends up being exported. I'll continue looking.

@tineheller
Copy link
Collaborator

The problem with aliases in modules can simply be fixed by adding case alias to case module in function prepareResults.

Any reason why these details are not used when identifying if/when things should be declared vs. exported?

Well we export by default everything that is not marked private. But you are right, that is not correct! We should rather use the export and the static flag. (We just added the behaviour for the private flag like JSDoc: @private but we forgot to change this case.)

I added the fixes here:
https://github.com/tineheller/jsdoc-tsd/blob/master/src/core/jsdoc-tsd-parser.ts

Additional I added two examples that use inner and exported functions in modules
https://github.com/tineheller/jsdoc-tsd/blob/master/exampleProject/src/core/myModule2.js
https://github.com/tineheller/jsdoc-tsd/blob/master/exampleProject/src/core/myModule3.js

I'm really sorry but I cannot merge and publish this before next week. So if you need it now, maybe you can work with local changes.

@whitlockjc
Copy link
Author

whitlockjc commented Aug 8, 2018

Your repository works fine. :) I've tested it and it works. Now to figure out why @returns {Promise<*>} becomes Promise.<*> instead of Promise.<any> and I'll be in business. (I already reported this in #38)

@wehrstedt
Copy link
Collaborator

ok, these fixes seems to work. Just to clarify it:
The function handleFlags is used to identify if a member has to be exported or not (or should be static or abstract). This function gets called here for every parsed item. Maybe this helps you to understand how scopes are mapped.

To describe a callback parameter, there are several ways to do this, and as often, there is no "right way". You can declare a function

/**
 * @callback
 */
function a() {
}

/**
 * @param {a} callback Callback function
 */
function b(callback) { }

or you can use arrow functions, for example:

/**
 * @param {() => string} callback Callback function
 */
function b(callback) { }

As @tineheller mentioned, the alias-case has to �be added to the case module part, for workspaces it works as expected. But it's good that you tested it with modules. I don't use modules that often (in javascript) so it's harder for me to find issues :)

@wehrstedt
Copy link
Collaborator

Fixed with #40

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

No branches or pull requests

3 participants