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

concat documentation not showing up on rxjs.dev #7009

Closed
carltheperson opened this issue Jul 2, 2022 · 4 comments · Fixed by #7069
Closed

concat documentation not showing up on rxjs.dev #7009

carltheperson opened this issue Jul 2, 2022 · 4 comments · Fixed by #7069

Comments

@carltheperson
Copy link

Describe the bug

Some awesome documentation exists in the file src/internal/observable/concat.ts as JSDoc.

However, it's not present on rxjs.dev.

What it currently looks like:
image

Expected behavior

I expect the documentation to show up on rxjs.dev.

It would look like this:

image

Reproduction code

# Non-local: 
- Go to: https://rxjs.dev/api/index/function/concat

# Local:
- cd docs_app
- npm run docs
- npm run start
- Go to: http://localhost:4200/api/index/function/concat

Reproduction URL

https://rxjs.dev/api/index/function/concat

Version

7.5.5

Environment

No response

Additional context

I have narrowed the problem down to the TypeScript package in dgeni-packages.

It produces wrong information about the function overloads. I should probably open an issue there, but I want to know what you think about this first. Maybe I'm totally off. Also, I think you'll be quicker to respond and might be interested in the temporary fix I have.

How to test this.

I wrote this function to debug this issue:

function analyzeTypes(filename, docs) {
  const relavantDocs = docs.reduce((acc, doc) => {
    if (!doc.fileInfo?.relativePath?.includes(filename)) {
      return acc;
    }

    return [
      ...acc,
      {
        id: doc.id,
        docType: doc.docType,
        type: doc.type,
        // Note: These line numbers don't seem to match *exactly* with the file.
        // Not sure why. They almost match though.
        startingLine: doc.startingLine,
        endingLine: doc.endingLine,
        overloads: doc.overloads?.map((doc) => doc.id),
      },
    ];
  }, []);
  console.log();
  console.log(`-- TYPE ANALYSIS ${filename} --`);
  console.log();
  console.log(relavantDocs);
  console.log();
}

It can be defined and called in docs_app/tools/transforms/angular-api-package/processors/filterContainedDocs.js. Here is where and how I call it:

// ...
$process: function (docs) {
      var docTypes = this.docTypes; // line 11
      analyzeTypes('internal/observable/concat.ts', docs);
// ...

The key information it logs is the docType of the docs array elements. Context: filterContainedDocs filters away certain docTypes; One being 'function-overload'

Current output of my function:

[
  {
    id: 'index/concat',
    docType: 'function',
    type: '',
    startingLine: 5,
    endingLine: 6,
    overloads: [ 'index/concat()', 'index/concat(...args: any[])' ]
  },
  {
    id: 'index/concat()',
    docType: 'function-overload',
    type: '',
    startingLine: 7,
    endingLine: 7,
    overloads: undefined
  },
  {
    id: 'index/concat(...args: any[])',
    docType: 'function-overload',
    type: 'Observable<unknown>',
    startingLine: 10,
    endingLine: 114,
    overloads: undefined
  },
  {
    id: 'index/concat~args',
    docType: 'parameter',
    type: 'any[]',
    startingLine: 113,
    endingLine: 112,
    overloads: undefined
  }
]

Notice the third element with id index/concat(...args: any[]). It got a docType of 'function-overload' (clearly wrong. It's not an overload) resulting in it being filtered away.

Quickfix

I'm still not exactly sure why this happens. I do know one fast way to fix it though: change the order of the overloads.

src/internal/observable/concat.ts contains two overloads at the beginning of the file. If we move them to the bottom of the file we now get another output from my function -

Quickfix output of my function:

[
  {
    id: 'index/concat',
    docType: 'function',
    type: 'Observable<unknown>',
    startingLine: 5,
    endingLine: 109,
    overloads: [ 'index/concat()', 'index/concat()' ]
  },
  {
    id: 'index/concat~args',
    docType: 'parameter',
    type: 'any[]',
    startingLine: 108,
    endingLine: 107,
    overloads: undefined
  },
  {
    id: 'index/concat()',
    docType: 'function-overload',
    type: '',
    startingLine: 110,
    endingLine: 111,
    overloads: undefined
  },
  {
    id: 'index/concat()',
    docType: 'function-overload',
    type: '',
    startingLine: 112,
    endingLine: 112,
    overloads: undefined
  }
]

It's now correct! (this produces the beautiful docs in the screenshot from "Expected behavior" )

I would be more than happy to submit a PR with this fix. I think a few other operators has this problem. I could run through them all to make sure the ordering allows for expected docs.

@jakovljevic-mladen
Copy link
Member

Also, I think you'll be quicker to respond and might be interested in the temporary fix I have.

Oh, I really hope that our quick response wasn't too late 😄

I wonder how this works in every other override we have? 🤔

E.g. share() operator also has two overloads: share<T>() and share<T>(options: ShareConfig<T>), but it seems that it works as expected. Both of them are declared above the function implementation and JSDocs.

Can you please write here the output of your analyzeTypes for share operator?

Thanks a lot for the report and thorough explanation of the issue.

@carltheperson
Copy link
Author

Found two other operators with this problem. This is the list so far:

I suspect the issue might have something to do with spread parameters.

analyzeTypes results

concat

We want id: 'index/concat(...args: any[])', to be of type function

[
  {
    id: 'index/concat',
    docType: 'function',
    type: '',
    startingLine: 5,
    endingLine: 6,
    overloads: [ 'index/concat()', 'index/concat(...args: any[])' ]
  },
  {
    id: 'index/concat()',
    docType: 'function-overload',
    type: '',
    startingLine: 7,
    endingLine: 7,
    overloads: undefined
  },
  {
    id: 'index/concat(...args: any[])',
    docType: 'function-overload',
    type: 'Observable<unknown>',
    startingLine: 10,
    endingLine: 114,
    overloads: undefined
  },
  {
    id: 'index/concat~args',
    docType: 'parameter',
    type: 'any[]',
    startingLine: 113,
    endingLine: 112,
    overloads: undefined
  }
]
bindNodeCallback

We want id: 'index/bindNodeCallback(callbackFunc: (args_0: any[], args_1: (err: any, ...res: any) => void) => void, resultSelector?: SchedulerLike | ((...args: any[]) => any), scheduler?: SchedulerLike)', to be of type function

[
  {
    id: 'index/bindNodeCallback',
    docType: 'function',
    type: '',
    startingLine: 10,
    endingLine: 12,
    overloads: [
      'index/bindNodeCallback(callbackFunc: (...args: any[]) => void, resultSelector: (...args: any[]) => any, scheduler?: SchedulerLike)',
      'index/bindNodeCallback(callbackFunc: (args_0: any[], args_1: (err: any, ...res: any) => void) => void, resultSelector?: SchedulerLike | ((...args: any[]) => any), scheduler?: SchedulerLike)'
    ]
  },
  {
    id: 'index/bindNodeCallback(callbackFunc: (...args: any[]) => void, resultSelector: (...args: any[]) => any, scheduler?: SchedulerLike)',
    docType: 'function-overload',
    type: '(...args: any[]) => Observable<any>',
    startingLine: 4,
    endingLine: 9,
    overloads: undefined
  },
  {
    id: 'index/bindNodeCallback~callbackFunc',
    docType: 'parameter',
    type: '(...args: any[]) => void',
    startingLine: 6,
    endingLine: 6,
    overloads: undefined
  },
  {
    id: 'index/bindNodeCallback~resultSelector',
    docType: 'parameter',
    type: '(...args: any[]) => any',
    startingLine: 7,
    endingLine: 7,
    overloads: undefined
  },
  {
    id: 'index/bindNodeCallback~scheduler',
    docType: 'parameter',
    type: 'SchedulerLike',
    startingLine: 8,
    endingLine: 8,
    overloads: undefined
  },
  {
    id: 'index/bindNodeCallback(callbackFunc: (args_0: any[], args_1: (err: any, ...res: any) => void) => void, resultSelector?: SchedulerLike | ((...args: any[]) => any), scheduler?: SchedulerLike)',
    docType: 'function-overload',
    type: '(...args: any[]) => Observable<any>',
    startingLine: 16,
    endingLine: 127,
    overloads: undefined
  },
  {
    id: 'index/bindNodeCallback~callbackFunc',
    docType: 'parameter',
    type: '(args_0: any[], args_1: (err: any, ...res: any) => void) => void',
    startingLine: 122,
    endingLine: 122,
    overloads: undefined
  },
  {
    id: 'index/bindNodeCallback~resultSelector',
    docType: 'parameter',
    type: 'SchedulerLike | ((...args: any[]) => any)',
    startingLine: 123,
    endingLine: 123,
    overloads: undefined
  },
  {
    id: 'index/bindNodeCallback~scheduler',
    docType: 'parameter',
    type: 'SchedulerLike',
    startingLine: 124,
    endingLine: 124,
    overloads: undefined
  }
]

race

We want id: 'index/race(...sources: (Observable<T> | InteropObservable<T> | AsyncIterable<T> | PromiseLike<T> | ArrayLike<T> | Iterable<T> | ReadableStreamLike<T> | ObservableInput<T>[])[])', to be of type function

[
  {
    id: 'index/race',
    docType: 'function',
    type: '',
    startingLine: 7,
    endingLine: 8,
    overloads: [
      'index/race()',
      'index/race(...sources: (Observable<T> | InteropObservable<T> | AsyncIterable<T> | PromiseLike<T> | ArrayLike<T> | Iterable<T> | ReadableStreamLike<T> | ObservableInput<T>[])[])'
    ]
  },
  {
    id: 'index/race()',
    docType: 'function-overload',
    type: '',
    startingLine: 9,
    endingLine: 9,
    overloads: undefined
  },
  {
    id: 'index/race(...sources: (Observable<T> | InteropObservable<T> | AsyncIterable<T> | PromiseLike<T> | ArrayLike<T> | Iterable<T> | ReadableStreamLike<T> | ObservableInput<T>[])[])',
    docType: 'function-overload',
    type: 'Observable<any>',
    startingLine: 10,
    endingLine: 54,
    overloads: undefined
  },
  {
    id: 'index/race~sources',
    docType: 'parameter',
    type: '(Observable<T> | InteropObservable<T> | AsyncIterable<T> | PromiseLike<T> | ArrayLike<T> | Iterable<T> | ReadableStreamLike<T> | ObservableInput<T>[])[]',
    startingLine: 51,
    endingLine: 50,
    overloads: undefined
  }
]

share (works)

We want id: 'operators/share(options: ShareConfig<T>)', to be of type function

[
  {
    id: 'index/share',
    docType: 'function',
    type: 'MonoTypeOperatorFunction<T>',
    startingLine: 52,
    endingLine: 238,
    overloads: [ 'index/share()', 'index/share(options: ShareConfig<T>)' ]
  },
  {
    id: 'index/share~options',
    docType: 'parameter',
    type: 'ShareConfig<T>',
    startingLine: 144,
    endingLine: 143,
    overloads: undefined
  },
  {
    id: 'index/share()',
    docType: 'function-overload',
    type: 'MonoTypeOperatorFunction<T>',
    startingLine: 48,
    endingLine: 49,
    overloads: undefined
  },
  {
    id: 'index/share(options: ShareConfig<T>)',
    docType: 'function-overload',
    type: 'MonoTypeOperatorFunction<T>',
    startingLine: 50,
    endingLine: 51,
    overloads: undefined
  },
  {
    id: 'index/share~options',
    docType: 'parameter',
    type: 'ShareConfig<T>',
    startingLine: 52,
    endingLine: 51,
    overloads: undefined
  },
  {
    id: 'operators/share',
    docType: 'function',
    type: 'MonoTypeOperatorFunction<T>',
    startingLine: 52,
    endingLine: 238,
    overloads: [ 'operators/share()', 'operators/share(options: ShareConfig<T>)' ]
  },
  {
    id: 'operators/share~options',
    docType: 'parameter',
    type: 'ShareConfig<T>',
    startingLine: 144,
    endingLine: 143,
    overloads: undefined
  },
  {
    id: 'operators/share()',
    docType: 'function-overload',
    type: 'MonoTypeOperatorFunction<T>',
    startingLine: 48,
    endingLine: 49,
    overloads: undefined
  },
  {
    id: 'operators/share(options: ShareConfig<T>)',
    docType: 'function-overload',
    type: 'MonoTypeOperatorFunction<T>',
    startingLine: 50,
    endingLine: 51,
    overloads: undefined
  },
  {
    id: 'operators/share~options',
    docType: 'parameter',
    type: 'ShareConfig<T>',
    startingLine: 52,
    endingLine: 51,
    overloads: undefined
  }
]

Solution

Putting the overloads below the actual function works like a charm. I would be happy to open up a PR with this @jakovljevic-mladen ? Let's get all this great docs back! Otherwise, I can open an issue on dgeni-packages.

I'll leave you with some before and after pics.

Before

image

image

image

After

image

image

image

@jakovljevic-mladen
Copy link
Member

jakovljevic-mladen commented Sep 7, 2022

Putting the overloads below the actual function works like a charm.

Hmm, I wanted to try it myself before writing anything. I was afraid of the TS wanting to have overload implementation in the last overload declaration, which is the case here. race produces another error if we put the two overloads from top to bottom:

image

The same happens with concat.

Also, race doesn't have proper types. The proper type includes ObservableInput which in case of race gets unwound to Observable<T> | InteropObservable<T> | AsyncIterable<T> | PromiseLike<T> | ArrayLike<T> | Iterable<T> | ReadableStreamLike<T> which is not correct either. Please take a look:

image

I'm not against having this issue temporarily fixed, but I'm afraid that this fix won't work due to TS errors.

Is there a way to open an issue on dgeni-packages, link this issue there and tag me so I can follow up?

Thanks a lot for bringing this up to our attention.

@jakovljevic-mladen
Copy link
Member

The fix is now deployed to the rxjs.dev app. Thanks for reporting.

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

Successfully merging a pull request may close this issue.

2 participants