Skip to content

Conversation

@craicoverflow
Copy link

@craicoverflow craicoverflow commented Sep 27, 2019

Closes #2183

Manually added an @internal JSDoc tag to exported functions that are not part of the public API.

I began by by making an ESLint rule which lives in the resources directory.

Although this helped me to identify the exported functions which are not exported in src/index.js, the rule is not yet stable enough to be included and I may look to follow this up with a separate PR, dealing exclusively with the ESLint rule.

@craicoverflow craicoverflow force-pushed the style/annotate-internal-functions branch from 74da857 to 27b8641 Compare September 27, 2019 15:26
@craicoverflow craicoverflow marked this pull request as ready for review September 27, 2019 15:39
@wtrocki
Copy link
Contributor

wtrocki commented Sep 27, 2019

I can confirm the same values locally. I just did not add internal flags to src/__tests__/starWarsData.js

/**
* Allows us to query for a character's friends.
*
* @internal
Copy link
Member

Choose a reason for hiding this comment

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

@craicoverflow Thanks a lot for PR 👍
One I didn't mention is that __tests__ & __fitures__ are not part of NPM package so all function exported from these folders are private by definition.
So can you please revert all changes under __tests__ folder?

/**
* Given a Path and a key, return a new Path containing the new key.
*
* @internal
Copy link
Member

Choose a reason for hiding this comment

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

Same goes for jsutils folder:
https://github.com/graphql/graphql-js/blob/master/src/jsutils/README.md

These functions are not part of the module interface and are subject to change.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I can change that 👍

It's maybe worth noting that in this same file, pathToArray is exported as public in src/index.js (aliased as responsePathAsArray).

responsePathAsArray,

Copy link
Member

Choose a reason for hiding this comment

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

@craicoverflow Good catch 👍 I need to think about it 🤔 💭

@craicoverflow craicoverflow force-pushed the style/annotate-internal-functions branch from 20c91a7 to 1f835bb Compare September 30, 2019 07:43
@craicoverflow craicoverflow force-pushed the style/annotate-internal-functions branch from 1f835bb to 88b4aa8 Compare September 30, 2019 07:45
@IvanGoncharov IvanGoncharov added the PR: polish 💅 PR doesn't change public API or any observed behaviour label Oct 1, 2019
@IvanGoncharov IvanGoncharov changed the title style(JSDoc): document private functions with @internal tag Document private functions with @internal tag Oct 1, 2019
@IvanGoncharov IvanGoncharov merged commit 5eb7c4d into graphql:master Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: polish 💅 PR doesn't change public API or any observed behaviour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Annotate all internal functions with '@internal'

3 participants