gcanti / docs-ts

A zero-config documentation tool for my TypeScript projects
https://gcanti.github.io/docs-ts/
MIT License
101 stars 17 forks source link

Improvements on the generated signature #13

Closed gillchristian closed 4 years ago

gillchristian commented 4 years ago

I believe the current output of the signatures could be improved to help readability.

I'm willing to work on these improvements, but first I'd like to see if this is something that would make sense to add.

So far I've found two cases that I'd like to improve.

NOTE: the suggestions are based on the goal of having the signature snippets being valid TS. See https://github.com/gcanti/docs-ts/issues/13#issuecomment-608373026

Only types in the signature

Currently the generated signature could be very verbose, as are TypeScript types. But it also includes part of the body of the function on it.

Eg. The following function from a RemoteData module that I use at work:

export const toEither = <E = unknown>(
  onNone: (status: 'idle' | 'loading') => E,
  onFailure: (err: ErrorAsync) => E
) => <A = unknown>(rda: RemoteData<A>): Either.Either<E, A> =>
  fold<A, Either.Either<E, A>>({
    idle: () => Either.left(onNone('idle')),
    loading: () => Either.left(onNone('loading')),
    success: Either.right,
    failure: flow(Either.left, Either.mapLeft(onFailure)),
  })(rda);

Produces this in the signature section.

export const toEither = <E = unknown>(
  onNone: (status: 'idle' | 'loading') => E,
  onFailure: (err: ErrorAsync) => E
) => <A = unknown>(rda: RemoteData<A>): Either<E, A> =>
  fold<A, Either<E, A>>({
    idle: () => left(onNone('idle')),
    loading: () => ...

Instead, only the types could be generated to show the signature.

declare const toEither: <E = unknown>(
  onNone: (status: 'idle' | 'loading') => E,
  onFailure: (err: ErrorAsync) => E
) => <A = unknown>(rda: RemoteData<A>) => Either<E, A>;

Overloaded functions

For the same RemoteData function, the filterOrElse function (from MonadThrow) produces this overloaded signature:

{ <E, A, B>(refinement: Refinement<A, B>, onFalse: (a: A) => E): (ma: RemoteData<A>) => RemoteData<B>; <E, A>(predicate: Predicate<A>, onFalse: (a: A) => E): (ma: RemoteData<A>) => RemoteData<A>; }

It's not very friendly to read the whole signature in one line like that.

Interface could be used for function overloading:

interface filterOrElse {
  <E, A, B>(refinement: Refinement<A, B>, onFalse: (a: A) => E): (
    ma: RemoteData<A>
  ) => RemoteData<B>;

  <E, A>(predicate: Predicate<A>, onFalse: (a: A) => E): (
    ma: RemoteData<A>
  ) => RemoteData<A>;
}
giogonzo commented 4 years ago

Interface could be used for function overloading:

The most important part of this improvement though is to split overloads on multiple lines

gillchristian commented 4 years ago

Indeed, the suggestion for using interfaces was to have the resulting type to be valid.

It could also be the approach of declare const:

declare const filterOrElse: <E, A, B>(
  refinement: Refinement<A, B>,
  onFalse: (a: A) => E
) => (ma: RemoteData<A>) => RemoteData<B>;

declare const filterOrElse: <E, A>(
  predicate: Predicate<A>,
  onFalse: (a: A) => E
) => (ma: RemoteData<A>) => RemoteData<A>;

But this snippet produces type errors since filterOrElse is declared twice.

I guess we could err on the side of pragmatisms and don't have the goal of making the signature snippets to be valid TS.

Other option that I can think of is to have several snippets (where each one is valid):

declare const filterOrElse: <E, A, B>(
  refinement: Refinement<A, B>,
  onFalse: (a: A) => E
) => (ma: RemoteData<A>) => RemoteData<B>;
declare const filterOrElse: <E, A>(
  predicate: Predicate<A>,
  onFalse: (a: A) => E
) => (ma: RemoteData<A>) => RemoteData<A>;
giogonzo commented 4 years ago

was to have the resulting type to be valid

I see, it makes sense

gcanti commented 4 years ago

Thanks @gillchristian

1) is a bug

2) I can add export declare const <const-name>, example:

export declare const filterOrElse: {
  <E, A, B>(refinement: Refinement<A, B>, onFalse: (a: A) => E): (ma: Either<E, A>) => Either<E, B>
  <E, A>(predicate: Predicate<A>, onFalse: (a: A) => E): (ma: Either<E, A>) => Either<E, A>
}

you can install the fix by running npm gcanti/docs-ts#13, could you please try it out?

gillchristian commented 4 years ago

Just tried it out, looks much better now.

gcanti commented 4 years ago

Thanks @gillchristian, released.