microsoft / tsdoc

A doc comment standard for TypeScript
https://tsdoc.org/
MIT License
4.71k stars 131 forks source link

DocFencedCode is mangled and does not necessary stringify to the original code, missing important characters #258

Closed phryneas closed 4 years ago

phryneas commented 4 years ago

So I noticed that when calling .toString() on the _codeExcerpt._content of a DocFencedCode, some characters seem to go missing.

See this example:

docNode._codeExcerpt.content.parserContext.sourceRange.toString()

/**
 * A utility function that allows defining a reducer as a mapping from action
 * type to *case reducer* functions that handle these action types. The
 * reducer's initial state is passed as the first argument.
 *
 * The body of every case reducer is implicitly wrapped with a call to
 * `produce()` from the [immer](https://github.com/mweststrate/immer) library.
 * This means that rather than returning a new state object, you can also
 * mutate the passed-in state object directly; these mutations will then be
 * automatically and efficiently translated into copies, giving you both
 * convenience and immutability.
 * @param initialState The initial state to be returned by the reducer.
 * @param builderCallback A callback that receives a *builder* object to define
 *   case reducers via calls to `builder.addCase(actionCreatorOrType, reducer)`.
 * @example
```ts
import {
  createAction,
  createReducer,
  AnyAction,
  PayloadAction
} from '@reduxjs/toolkit'
const increment = createAction<number>('increment')
const decrement = createAction<number>('decrement')

function isActionWithNumberPayload(
  action: AnyAction
): action is PayloadAction<number> {
  return typeof action.payload === 'number'
}

createReducer(
  {
    counter: 0,
    sumOfNumberPayloads: 0,
    unhandledActions: 0
  },
  builder =>
    builder
      .addCase(increment, (state, action) => {
        // action is inferred correctly here
        state.counter += action.payload
      })
      // You can chain calls, or have separate `builder.addCase()` lines each time
      .addCase(decrement, (state, action) => {
        state.counter -= action.payload
      })
      // You can apply a "matcher function" to incoming actions
      .addMatcher(isActionWithNumberPayload, (state, action) => {})
      // and provide a default case if no other handlers matched
      .addDefaultCase((state, action) => {})
)

When calling docNode._codeExcerpt._content.toString(), this is mangled to

import {
  createAction,
  createReducer,
  AnyAction,
  PayloadAction
} from '@reduxjs/toolkit'
const increment = createAction<number>('increment')
const decrement = createAction<number>('decrement')

function isActionWithNumberPayload(
  action: AnyAction
): action is PayloadAction<number> {
  return typeof action.payload === 'number'
-}

createReducer(
-  {
    counter: 0,
    sumOfNumberPayloads: 0,
    unhandledActions: 0
  },
  builder =>
    builder
      .addCase(increment, (state, action) => {
        // action is inferred correctly here
        state.counter += action.payload
      })
      // You can chain calls, or have separate `builder.addCase()` lines each time
      .addCase(decrement, (state, action) => {
        state.counter -= action.payload
      })
      // You can apply a "matcher function" to incoming actions
      .addMatcher(isActionWithNumberPayload, (state, action) => {})
      // and provide a default case if no other handlers matched
      .addDefaultCase((state, action) => {})
-)

(Diff highlight by me)

Actually, the same seems to happen in the TSdoc playground.

Just copy& paste my initial docblock over there: https://microsoft.github.io/tsdoc/ image

I guess this is a bug, or am I writing completely wrong tsdoc?

phryneas commented 4 years ago

So this seems to be a problem with the tokenizer removing all lines where only a single character is present. It seems possible to circumvent this by indenting with a *

So, this:

/**

a s d f g

 */

ends up as

but

/**
 * ```
 * a
 * s
 * d
 * f
 * g
 * ```
 */

works.

With that info, I can work around that in my comments for now, but this seems like a serious bug.

octogonz commented 4 years ago

This is the recommended standard way that it should be written:

/**
 * ```
 * a
 * s
 * d
 * f
 * g
 * ```
 */

But you are right, this is definitely a bug in the handling of the /** */ framing.

phryneas commented 4 years ago

Yeah, I wouldn't have noticed it if it weren't for the code block - but writing code blocks with leading * is really distracting from the code itself :/

octogonz commented 4 years ago

Yeah, I wouldn't have noticed it if it weren't for the code block - but writing code blocks with leading * is really distracting from the code itself :/

Makes sense. It does look like a pretty reasonable practice.

Here's a PR: https://github.com/microsoft/tsdoc/pull/259

phryneas commented 4 years ago

That was fast :) Thanks a lot!

By the way, if you are interested where your libs are used, here's my use case:

This is going to make it into the @reduxjs/toolkit docs. I'm writing a remark plugin that takes a link in the form of

[summary,remarks,params,examples](docblock://mapBuilders.ts?token=ActionReducerMapBuilder.addDefaultCase)

and renders the docblock into something like this (the TS code from the example is compiled down into JS for those tabs - in this case, TS and JS are identical though ^^)

octogonz commented 4 years ago

That is pretty cool!

octogonz commented 4 years ago

This fix was released with @microsoft/tsdoc version 0.12.21