krisztianb / typedoc-plugin-replace-text

Plugin for TypeDoc that replaces text in the documentation
ISC License
5 stars 2 forks source link

fix: hasSources returning false #10

Closed treardon17 closed 7 months ago

treardon17 commented 8 months ago

In testing the change for https://github.com/krisztianb/typedoc-plugin-replace-text/issues/9, I noticed the this.sources array was always undefined in the replacer function. Changing the hasSources function to check for the existence of the sources key rather than doing an instanceof check resolves the issue for me.

krisztianb commented 8 months ago

Thanks for reporting back. Was this happening when you were testing with the following do comment you posted in the issue thread?

/**
 * @packageDocumentation
 * README.MD
 */

If so this comment is probably associated with a different kind of reflection. I included all which had a "sources" property so I will have to investigate which type/class I missed.

treardon17 commented 8 months ago

@krisztianb yes, that is the type of comment I was testing with 👍

treardon17 commented 8 months ago

Out of curiosity, is there a reason you prefer instanceof vs in? Using in here will still do type narrowing, and it's a bit less code. Am I missing a reason to avoid in here?

krisztianb commented 8 months ago

Didn't know that TS can narrow down the type when using in. The only reason for me to use instanceof was to make sure that the return type (and the type guard) of the function is accurate.

treardon17 commented 8 months ago

Gotcha 👍 From what it looks like, both in and instanceof handle type narrowing https://www.typescriptlang.org/docs/handbook/2/narrowing.html#the-in-operator-narrowing. The only differences I can think of (outside of instanceof checking for the existence of a prototype in a prototype chain vs in checking for the existence of a property in the prototype chain) are that instanceof handles nullish things a bit differently than the in operator. For example, foo instanceof Bar would return false if foo was null or undefined whereas 'barProperty' in foo would throw an error if foo was null or undefined. Either way I think they both should work in this case.

However, I'm not entirely sure why instanceof is not working for me because it looks like there are only two types of reflections that are being used in my case: DeclarationReflection and SignatureReflection. The instanceof check never returns true (which confuses me), but the 'sources' in reflection check seems to consistently work.

krisztianb commented 8 months ago

Yes, that is odd. I will have to debug into this myself to figure out the problem.

krisztianb commented 8 months ago

I used your example and for me the reflection that has the @packageDocumentation comment is a ProjectReflection and not a DeclarationReflection. This explains why the instanceof checks fail for the reflection and you don't get any sources in your replacer function.

However according to the documentation of TypeDoc there shouldn't be any sources set on the ProjectReflection instance. I created an issue in the TypeDoc project to clearify what the expected behavior is: https://github.com/TypeStrong/typedoc/issues/2481

krisztianb commented 8 months ago

We got an answer from Gerrit (the lead developer of TypeDoc) who confirmed this (sources available on the ProjectReflection) being a bug. So it looks like that what you are trying to achieve will not work for certain comments. Nevertheless I could merge the branch so that it at least works for most comments. What do you think?

treardon17 commented 7 months ago

@krisztianb that sounds good to me! My build script creates a bunch of modules using the json option, and then ties them all together in a final build step, which might be why I have different reflection types than you do 👍