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

TypeScript class method decorator not rendering properly in version >= 1.63.2 #47679

Open
dreamorosi opened this issue Jan 31, 2022 · 18 comments
Assignees
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@dreamorosi
Copy link

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.63.2
  • OS Version: Darwin x64 20.6.0

Steps to Reproduce:

  1. Write a JSDoc string with an example that involves a decorator:
class MyClass {
   /**
   * This is an example of a decorator.
   * 
   * @example
   * ```typescript
   * import { MyClass } from '@org/myClass';
   * 
   * const myClass = new MyClass();
   * 
   * class MyAwesomeClass {
   *   @myClass.decorateMethod()
   *   public doStuff() {
   *     ...
   *   }
   * }
   * 
   * ```
   * 
   * @decorator Class
   */
   public decorateMethod(): MyType {
     return (target, _propertyKey, descriptor) => {
       const originalMethod = descriptor.value;

       descriptor.value = ((event, context, callback) => {
         return originalMethod?.apply(target, [ event, context, callback ]);
       });

       return descriptor;
     };
   }
}
  1. Use decorator / hover on method declaration:

image

As you can see from the screenshot above the example appears not to be rendered correctly with two main issues:

  • Decorator is rendered on a new line & wrapped with *
  • All code indentation after the decorator is lost

Before opening this issue I've searched the repo (& the TypeScript one) and found a series of other issues. Most of them have been closed as duplicates or as fixed but as the above shows the issue is still there. Examples:

If the issue has been indeed fixed, could you please take a second to clarify what's the intended usage to allow VS Code to render decorators correctly? I'm aware that decorators are an experimental feature, if that's the reason why this is not supported could you please state it?

Thanks in advance.

@mjbvz
Copy link
Contributor

mjbvz commented Feb 1, 2022

Looks like a duplicate of #35310 but let's wait for feedback from the TS team

@dreamorosi
Copy link
Author

None of the workarounds suggested in that issue work as it's also reported by other users in that same issue.

It was marked as "work as intended", does it mean that decorators are not supposed to be rendered correctly?

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 1, 2022
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.7.0 milestone Feb 1, 2022
@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label May 13, 2022
@dreamorosi
Copy link
Author

Since this was rescheduled/left out of the last two iterations, is there any chance that this can be added to the TypeScript 4.9.0 one?

@sandersn
Copy link
Member

sandersn commented Sep 6, 2022

Looking at the parser, the problem is that backticks don't persist across lines, even triple backticks.

@sandersn
Copy link
Member

sandersn commented Sep 6, 2022

A naive change to stop resetting the backtick parsing does stop treating @myClass as a tag, but also stops maintaining the indent margin and skipping leading asterisks. That needs to be maintained, even when parsing backtick-quoted text.

@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Sep 7, 2022
@sandersn
Copy link
Member

Update: the fix at #50677 correctly makes single and triple backticks multiline, but this breaks unclosed backticks in the tests and would break them in the compiler source code itself. We're discussing whether to take the change or not.

@trusktr
Copy link
Contributor

trusktr commented Oct 1, 2022

\` for single back ticks perhaps.

I don't think I've ever used or seen a single back tick anywhere, so a very few minority of people (if they even exist) can migrate.

@trusktr
Copy link
Contributor

trusktr commented Oct 1, 2022

I think a solution should work with both code fences, and @example blocks.

Personally I think GitHub has the best handling of back ticks. Might be worth looking at.

But if back ticks can't be fixed, I think at least \@ inside of an @example block (or anywhere) should just display @ as a single character.

Another option for escaping @ could be to write one double: @@.

JSDoc was designed before the language had decorators, so I think anything goes here really.

@trusktr
Copy link
Contributor

trusktr commented Oct 1, 2022

better-docs is a sweet JSDoc parser and generator for TypeScript, backward compatible with the original JSDoc.

https://github.com/SoftwareBrothers/better-docs

@wojtek-krysiak do you have any thoughts on how to handle decorators?

I think it would make sense to ensure that TypeScript's format aligns well with the best tools in the community (like better-docs).

Better docs is great because it takes TypeScript code (with JSDoc) and can output 100% actual-JSDoc compatible JSDoc comments for compatibility with official JSDoc tooling, whereas other tools like TSDoc are not compatible.

@trusktr
Copy link
Contributor

trusktr commented Oct 1, 2022

JSDoc itself is still active, and the community has open issues for this:

jsdoc/jsdoc#1521
jsdoc2md/jsdoc-to-markdown#209

@hegemonic do you have any opinions on decorator support?

I believe we should all aim to be on the same page to avoid community fracture. It would be so much better to coalesce into a common future.

In the worst case, a tool like better-docs would have to take incompatible syntax and convert it to compatible syntax, which wouldn't be ideal.

@dreamorosi
Copy link
Author

Hi everyone, any chance to get this addressed as part of the 5.0 milestone?

@HiEv
Copy link

HiEv commented Aug 11, 2023

Any progress on the code indentation portion of this issue?

@trusktr
Copy link
Contributor

trusktr commented Sep 25, 2023

Decorators are non-experimental now with TypeScript 5. Can we have this fix soon pleeease? 🙏

@rmnilin
Copy link

rmnilin commented Oct 16, 2023

This is really stupid and bad, but I use invisible separator U+2063 "⁣" before "@" as workaround and it works fine.

image

@trusktr
Copy link
Contributor

trusktr commented Nov 20, 2023

@RNLN I tried your suggestion, but that causes weird formatting for me:

Screenshot 2023-11-20 at 1 17 03 PM

For now, this is what I've settled with my docs looking like:

Screenshot 2023-11-20 at 1 13 52 PM

(note, replace double ampersands with one ampersand)

🤣

@rmnilin
Copy link

rmnilin commented Nov 21, 2023

@trusktr It's odd because I tried adding this invisible character to your documentation and it worked for me.

Screenshot image

After that, I took the liberty of adjusting the indentation, using @example instead of ```.

Screenshot image

This is just an example and I'm not sure it will be of help, but here's a snippet just in case:

Snippet
/**
 * @class Effectful -
 *
 * `mixin`
 *
 * Create Solid.js effects using `this.createEffect(fn)` and easily stop them
 * all by calling `this.stopEffects()`.
 *
 * @example
 * import {element, Effectful} from 'lume'
 *
 * ⁣@element('my-element')
 * class MyElement extends Effectful(HTMLElement) {
 *   ⁣@attribute foo = "foo"
 *   ⁣@attribute bar = "bar"
 *
 *   connectedCallback() {
 *     // Log `foo` and `bar` any time either of them change.
 *     this.createEffect(() => {
 *       console.log('foo, bar:', this.foo, this.bar)
 *     })
 *
 *     // Log only `bar` any time it changes.
 *     this.createEffect(() => {
 *       console.log('bar:', this.bar)
 *     })
 *   }
 *
 *   disconnectedCallback() {
 *     // Stop both of the effects.
 *     this.stopEffects()
 *   }
 * }
 */

@trusktr
Copy link
Contributor

trusktr commented Nov 29, 2023

That snippet is working for me now. Not sure why I wasn't able to get it working before. EDIT: Ah, I was putting the invisible non-space after the @, not before the @.

Also, weird, I thought indentation when using code fences worked before (might be mis-recalling, and here's a bug report for that), but I do see it is currently broken. Unfortunately I have to keep the code fences for now because those JSDocs get consumed and rendered as markdown, f.e. the content here is from JSDoc tags:

https://docs.lume.io/api/utils/Settable

Source:

https://github.com/lume/lume/blob/develop/src/utils/Settable.ts#L5

But perhaps I can update the JSDoc extractor to support @example. How do we use multiple @example blocks with non-code content (f.e. a paragraph) between each example like we can with code fences under a single JSDoc tag? Doesn't seem possible.

@Memoraike
Copy link

Memoraike commented Jan 19, 2024

@RNLN nice trick! <3 It didn't work for me, but it gave me a clue about the symbol "U+2064" (invisible plus). It's still a hack, but it's working :)

VSCode

2024-01-19_10-46

Jetbarins IDEA

2024-01-19_10-47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants