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.

Sign up for GitHub

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

Fixes to support Generating API Documentation #4391

Merged
merged 6 commits into from
Oct 27, 2023
Merged

Fixes to support Generating API Documentation #4391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
84143a9
StateValue: separate summary from remarks
audionerd Oct 19, 2023
5393ea9
use `@internal` instead of `@private`
audionerd Oct 23, 2023
decfcf7
NonReducibleUnknown: add `@remarks`
audionerd Oct 24, 2023
045fbb0
StateValue: fix list item rendering
audionerd Oct 25, 2023
cfa877e
StateNodeConfig.parent: remove comment
audionerd Oct 26, 2023
c9e9b6e
microstep: omit `@internal` and comments
audionerd Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions packages/core/src/stateUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,11 +982,6 @@ function areConfigurationsEqual(

/**
* https://www.w3.org/TR/scxml/#microstepProcedure
*
* @private
* @param transitions
* @param currentState
* @param mutConfiguration
*/
export function microstep<
TContext extends MachineContext,
Expand Down
9 changes: 6 additions & 3 deletions packages/core/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ export type HomomorphicOmit<T, K extends keyof any> = {
};

/**
*
* @remarks
*
* `T | unknown` reduces to `unknown` and that can be problematic when it comes to contextual typing.
* It especially is a problem when the union has a function member, like here:
*
Expand Down Expand Up @@ -284,7 +287,10 @@ export interface StateValueMap {
/**
* The string or object representing the state value relative to the parent state node.
*
* @remarks
Copy link
Member

Choose a reason for hiding this comment

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

I'm still quite unsure why we need this. RxJS has extensive and long JSDocs and they use api-extractor too but there is no @remarks in sight in their codebase. I wonder what's the difference? 🤔

Copy link
Contributor Author

@audionerd audionerd Oct 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

On the index page a description is provided for each item.

With @remarks, we can choose to show only the first summary line in the descriptions:
index-with-remarks

Without @remarks, the description is longer (and most of the index descriptions are a single line so this one stands out):
index-without-remarks

So it’s a subjective change, but I thought it was better to have a shorter description. If you disagree I’m happy to change it back.

I had also noticed the list items weren’t formatted properly on the index, so I wanted to omit them, but I just found a fix for that I can push to this PR, so it's not a deciding factor.

Copy link
Contributor Author

@audionerd audionerd Oct 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

According to the API Extractor docs, it expects TSDoc and @remarks is specific to TSDoc, which explains why you wouldn’t see it in JSDocs. See: https://tsdoc.org/pages/tags/remarks

I’m curious about how RxJS docs are set up though. I’ll take a look.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, OK - so actually RxJS is not using the api-extractor (my memory is failing me). I agree that a shorter summary in this table is better but I really wish that we wouldn't have to use @remarks for this. Despite TSDoc being a Microsoft project, VS Code doesn't handle those TSDoc tags nicely 🤷‍♂️

I'm fine either way though - we can always revisit this in the future. I just feel that the primary use case for those JSDoc comments is the in-IDE experience and the API docs page generation comes second. We have some level of control over how API docs end up being generated as we can script whatever we want - we have 0 control over how an IDE renders its tooltips (beyond what we can configure using the JSDoC tags that it knows and understands)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. I’ll see what I can find out about TSDocs in VS Code. Might be some ways to improve the experience.

*
* - For a child atomic state node, this is a string, e.g., `"pending"`.
*
* - For complex state nodes, this is an object, e.g., `{ success: "someChildState" }`.
*/
export type StateValue = string | StateValueMap;
Expand Down Expand Up @@ -834,9 +840,6 @@ export interface StateNodeConfig<
TGuard,
TDelay
>;
/**
* @private
*/
parent?: StateNode<TContext, TEvent>;
/**
* The meta data associated with this state node, which will be returned in State instances.
Expand Down