Annotate all internal functions with '@internal'

[IvanGoncharov]

Even though some functions are expored from src/**/*.js files they are private. The only function exported by src/index.js and src/*/index.js are considered to be part of the public API. Please see: https://github.com/graphql/graphql-js/blob/master/src/README.md

It would be great to have all private API marked as such, e.g.: https://github.com/graphql/graphql-js/blob/6f341a3ae0d6b3cc4d9136a58fd9272c615c2dba/tstypes/validation/validate.d.ts#L32-L33

Also, it would be great to find all public APIs without JS doc comments.

[wtrocki]

Working on it. Removed assignment by mistake

[wtrocki]

Spent the entire day on this and doing this by hand is really tricky and prone to errors. I could do something fast but I would not be content about every change there and verification would be also tricky as well.

How to resolve this problem

My approach for this would be to:

• Build a unit test/script (after migration to ts it will be ts-lint rule) that will read the entire exported api from the index.
• Then it will traverse thru the source code and compare individual index.js exports (and individual files)

• Build diff and print it (later it could just produce a warning

  There is the interest of having this kind of check-in TypeScript community (Exported member is not available globally) and we can configure comment format that will disable that as a warning.

Advantage

Advantage of this approach is that:

• It could be enforced at review time which safes mental effort to review PR's
• It will be not TypeScript/Flow specific
• It will reduce the tedious job of asking people to do it for every PR - when introducing new changes
• Having ability to replicate this fast will mean that there is a lesser chance of doing the same manual job twice when source code will be in typescript.

What it means that it will require much more work initially, but it will be much better for tracking API changes in the future. @IvanGoncharov

I have seen some babel tools that can help here, but if you guys know any other tools that can help feel free to post it here.

[IvanGoncharov]

@wtrocki tslint is in deprecated state, you can write a custom ESlint and put it in the resources directory.

[wtrocki]

Yes. I meant ESLint. For the moment I just have node script that need to print out method names so I can find them and manually add comment

[wtrocki]

Got some progress on this but still not ready for PR. I will be off next week so hopefuly I will manage to create PR next next week

[IvanGoncharov]

@craicoverflow Thanks a lot for #2205 I merged it in but I will keep this issue open to tracking the development of ESLint rule. Another great subtask would be to check that all public functions have doc comments.

[nveenjain]

@IvanGoncharov , related to the development of ESLint Rule, I was thinking we can do it as 2 step process:-

1. Get list of exported functions/classes from the index files...
2. Compare exported function/class name in the current file with that of the exported functions/classes from the index and generate an error if internal JSDoc comment is not present...

We can execute the first step through babel plugin to get list of all the exported variables from the index in current subdirectory of Graphql... For the second step we can search for the value of the name of function/class in the list generated... If not present, current function must have internal annotated...

This might cause a problem when 2 different files export the function/class having same name... We can tackle this problem in 2 ways:-

1. We must ensure that this never happens and exported names are always different for different files atleast under same sub-directory in graphql (This will be easier to code)
2. Add the source too in the list generated and use it in second step... (A little bit complicated to code)

Can I take up this issue, if yes, which approach should I follow?

[nveenjain]

I created a babel-plugin for the same since we are using only ExportNamedDeclarations, this plugin only extracts the list of exported variables with ExportNamedDeclarations... This means that code like

 export let name1, name2=5, nameN;

is not supported, and name1, name2, nameN will not be output of the plugin... However this can be modified if required... You can check the plugin live at:- https://astexplorer.net/#/gist/01dc219b927b6a72efc1ca00a92fa0be/35b47cf3831059793f993bbd3a4d71b140759972

You can check the console to get the array of exported names/functions...