Ignore / exclude props from output

[JakeSidSmith]

Hi there,

I'm working on a large component library that has to be very flexible, and therefore am defining my props in the following way to allow all the standard props, should users want to provide them:

export interface ButtonProps extends HTMLProps<HTMLButtonElement> {
  component?: string;
  block?: boolean;
  large?: boolean;
  small?: boolean;
}

Using this with styleguidist generates a huge list of props. Is there some way I can exclude these from the output? If not, would such a thing be possible?

Example output:

I'd be happy to try to add this feature, as well as fixing some of the other open issues.

[JakeSidSmith]

Alternatively, an option to exclude props that do not have a description, or additional flag in the prop's comment would be useful.

[Inoir]

Any updates on this?

[JakeSidSmith]

Haven't had a chance to look into this properly myself yet @Inoir. Will update this issue if I make any progress.

[pvasek]

Maybe, we can try to define how it should work first. I checked http://usejsdoc.org/ but it doesn't seem there is anything similar.

Any ideas? Something like @ignoreparentprops?

[JakeSidSmith]

@pvasek you might not want to ignore all props that you've extended though, just the React ones.

I think the best thing might be only displaying props that have a doc comment.

[JakeSidSmith]

Some of my components have a common interface that they extend, but also HTMLProps.

import { HTMLProps } from 'react';

interface Common {
  /**
   * I should be documented
   */
  common: number;
}

interface Props extends Common, HTMLProps<HTMLElement> {
  /**
   * I should be documented
   */
  custom: string;
}

[besrabasant]

@JakeSidSmith

This is a workaround that I am using.

type StringLiteralDiff<T extends string, U extends string> = ({[P in T]: P } & {[P in U]: never } & { [x: string]: never })[T];

type Omit<T, K extends keyof T> = Pick<T, StringLiteralDiff<keyof T, K>>;

interface ComponentProps {
    propA: string
    propB: number
    propC: () => void
}

interface ExtendedComponentProps extends Omit<ComponentProps, 'propC'> {
    // only 'propA' and 'propB' are included here.
}

[JakeSidSmith]

@besrabasant thanks, but my issue is that I want to exclude all of the HTMLProps, some of which may have the same keys as my custom props.

Additionally this would remove them from my components interface. 🙁

[besrabasant]

@JakeSidSmith You could something like this.

export interface ButtonProps extends Omit<HTMLProps<HTMLButtonElement>,'propNameto be removed'> {
  component?: string;
  block?: boolean;
  large?: boolean;
  small?: boolean;
}

[besrabasant]

@JakeSidSmith
If you want to remove multiple properties from Parent Interface, you can do something like this.

export interface ButtonProps extends Omit<HTMLProps<HTMLButtonElement>,'propName1' | 'propsName2' | 'propName3' | '...' > {
  component?: string;
  block?: boolean;
  large?: boolean;
  small?: boolean;
}

[JakeSidSmith]

@besrabasant I don't want to exclude any props from the component interface, just from the documentation generated, as they will be the same for all components.

[wachunga]

I also have need of this. In my case, we often use &.

export type CheckboxProps = RebassProps &
  InputHTMLAttributes<HTMLInputElement> & {
    label?: string;
  };

I would be in favour of @JakeSidSmith's idea to only display props that have a docgen comment. How about config skipPropsWithoutDoc akin to skipComponentsWithoutExample?

[pvasek]

@wachunga I really like your proposal. It seems it could solve most of these problems and it is simple enough to implement. Just filter out properties without docs.

[Havret]

I am trying to implement this one, but got blocked on figuring out what is the best way to pass flag to the parser. Current API seems to be too rigid and hard to extend in a easy way.

Here is the failing test:
https://github.com/Havret/react-docgen-typescript/tree/feature/skipPropsWithoutDoc

[wachunga]

I had a quick look too. Here's Havret's diff. Any suggestions, @pvasek?

[Havret]

I think we could change our API to sth like withOptions() which would accept options object with all properties that we need.

@pvasek what do you think about it?

[pvasek]

There is pull request #63 from @dotcs. He tries to solve different thing but he already implemented passing options all the way down to the parser.

It seems that everyone wants filter properties based on different requirements. Maybe it would make sense to have just propsFilter: prop => boolean option. Then you could define options like

{  propsFilter: prop => prop.description.length; }

for filtering properties without docs or

{  propsFilter: prop => prop.description.length && prop.name === "children"; }

for removing children prop without comment as in his pull request.

Of course this could be improved by creating helper filters directly in react-docgen-typescript then we could have:

{  propsFilter: onlyPropsWithComment }

What do you think about it?
Is it better to have something general or just set of fixed booleans?

[Havret]

I think the general idea is good but I would simplify api and get rid of withDefaultConfig, withCustomConfig, withCompilerOptions.

[JakeSidSmith]

Just implemented ignoring props without descriptions. As an additional part of this branch I could add ignoring specific props, defined in a list?

This keeps things very flexible.

Pull request

[JakeSidSmith]

And I've named the config option incorrectly. 😆 Give me a couple of minutes.

[Havret]

I think we should follow the direction pointed by @pvasek and implement this as filtering function. It would be much more flexible. Of course we can provide some basic filtering rules out of the box. :)

[JakeSidSmith]

I can easily tweak it to take a function. I think having a couple of options for users that want to easily ignore specific props / those without descriptions would also be useful though.

[JakeSidSmith]

Okay, I've updated with:

export interface ParserOptions {
    skipPropsWithName?: string[] | string;
    skipPropsWithoutDoc?: boolean;
    propsFilter?: (jsDocComment: JSDoc) => boolean;
}

[dotcs]

I like the idea pointed out by @pvasek to use a function for props filtering. That would allow to generalise my approach in PR #63.

But I think it would be best to pass the function also information about the component. It could be that the propsFilter depends on for example the name of the component. So im my opinion it would be nice to have something like propsFilter: (prop, component) => boolean.

Also in my opinion we should limit this to only the propsFilter method and not add additional options that allow to interfere such as skipPropsWithName, skipPropsWithoutDoc because which of those options wins in case they contradict each other?

[dotcs]

I've updated PR #63 and implemented the ideas of pvasek.

[pvasek]

Regarding the propsFilter definition, I agree with @dotcs that there should more then less. For now the component has only the name but it would probably make no harm to pass it there. Maybe typings could look something like this:

export interface Prop {
    name: string;
    required: boolean;
    type: PropItemType;
    description: string;
    defaultValue: any;
}

export interface Component {
   name: string;
}

filter definition would be:

(prop: Prop, component?: Component) => boolean

but in general, maybe this is really "too" complicated and simple options would work for most cases as @JakeSidSmith proposed. About question how they will work together we can have have an array of these filters internally and each one would be applied. Or maybe the ParseOptions could be:

export interface StaticPropFilter {
    skipPropsWithName?: string[] | string;
    skipPropsWithoutDoc?: boolean;
}

export interface ParserOptions {
    propsFilter?: StaticPropFilter | (prop: Prop, component?: Component) => boolean;
}

In that case you would be able to use one of the build-in or create your own.

[dotcs]

I like dividing propsFilter into one or the other. This would definitely keep things clean and simple. As long as there are not too much of them, it also does not affect computation time too much.

In my PR added

export type PropsFilter = (props: PropItem & PropItemType, componentName: string) => boolean;

so that we could write

export interface ParserOptions {
    propsFilter?: StaticPropFilter | PropsFilter
}

I will rename PropsFilter to PropFilter, to keep it aligned with StaticPropFilter. I can also add interfaces for Prop and Component. Should we also divide in StaticPropFilter | PropsFilter in this PR or should we do this in another PR? @JakeSidSmith's has done most of the work of the static filters, I guess.

[sapegin]

@dotcs I like your current implementation, I think it would make sense to have the most useful default implementation that would at least skip undocumented children.

[dotcs]

Thanks @sapegin.

We could skip undocumented children by using this default implementation

propFilter: (prop, component) => prop.name !== 'children' || (prop.name === 'children' && prop.description.length > 0)

Would this be fine for everybody?

[Havret]

Not really. If I pass my custom filter it will override the default one and children magically pop back. I would bake this inside parser implementation.

[pvasek]

In that case we should go back to @JakeSidSmith suggestion. In this version with tiny filter improvement.

export interface ParserOptions {
    skipPropsWithName?: string[] | string;
    skipPropsWithoutDoc?: boolean;
    propsFilter?: (props: Prop, component: Component) => boolean;
}

and internally have an array of filters which can be build based on these options.

[pvasek]

oh sorry, missed that children option:

export interface ParserOptions {
    skipUndocumentedChildren?: boolean;
    skipPropsWithName?: string[] | string;
    skipPropsWithoutDoc?: boolean;
    propsFilter?: (props: Prop, component: Component) => boolean;
}

[dotcs]

I've updated my PR. It now implements filtering via function and static approaches.

Filtering of uncommented children is baked in and cannot be toggled via skipPropsWithoutDoc as suggested by @sapegin. This matches the behaviour of the JavaScript parser.

export type PropFilter = (props: PropItem, component: Component) => boolean;

export interface StaticPropFilter {
  skipPropsWithName?: string[] | string;
  skipPropsWithoutDoc?: boolean;
}

export interface ParserOptions {
  propFilter?: StaticPropFilter | PropFilter
}

The static filter can either skip one or more properties with skipPropsWithName or skip any props that don't come with a doc via skipPropsWithoutDoc.

[pvasek]

@dotcs Thanks for your PR. Released as v1.2.2.

[tuchk4]

This solutions doesn't work for me. All HTMLButtonElement props with descriptions so I can't filter them.

interface IButtonProps extends React.HTMLAttributes<HTMLButtonElement> {
  color?: string;
}

const Button: React.SFC<IButtonProps> = ({ color }) => {
  // ...
  return (
    <button>
      {children}
    </button>
  );
};

Or I do something wrong :) Because there are no comments after issue was closed.

[tuchk4]

All props at @types/react https://unpkg.com/@types/react@16.4.7/index.d.ts with descriptions.
Can't understand how can I filter them with propFilter function.

@pvasek @dotcs

[Havret]

@tuchk4 You can filter them like that:

react-docgen-typescript/examples/react-styleguidist-example/styleguide.config.js

Line 13 in 57bf7ae

propsParser: require('react-docgen-typescript').withDefaultConfig({ propFilter: { skipPropsWithoutDoc: true } }).parse

Here you can check out working example:
https://github.com/Havret/material-components-react

[tuchk4]

I already add propsParser with { propFilter: { skipPropsWithoutDoc: true } } but as I wrote above - typings at @types/react are with description so they will not be filtered.

@Havret your example works because you use ObjectOmit

• Checkbox example https://github.com/Havret/material-components-react/blob/master/src/Checkbox/Checkbox.tsx#L5
• ObjectOmit implementation https://github.com/Havret/material-components-react/blob/master/src/utils/typeHelper.ts

I tried to do the same but got error:

TS2344: Type 'keyof O' does not satisfy the constraint 'string'.
  Type 'string | number | symbol' is not assignable to type 'string'.
    Type 'number' is not assignable to type 'string'.

1. Is there is any way to filter props without ObjectOmit?
2. Why I got error with ObjectOmit?

[Havret]

The only thing ObjectOmit does in this example is removing "type" property from the type HTMLInputElement, thus is makes rather minor impact on resulting docs.

Could you provide some repro example with the problem you are facing?

[tuchk4]

Here is example repo - https://github.com/tuchk4/sg-ts-filter-props

As I wrote above - types at @types/react with descriptions so they will not be filtered with propFilter.

[tuchk4]

Another solution I found is updateDocs function. But it is not perfect. Still looking for good solution to filter React.HTMLAttributes<HTMLButtonElement> props

const path = require('path');
const isReactProp = require('is-react-prop').default;

const NO_FILTER_PROPS = ['width', 'height', 'margin', 'padding', 'color'];

module.exports = {
  title: 'Test',

  resolver: require('react-docgen').resolver.findAllComponentDefinitions,
  propsParser: require('react-docgen-typescript').withDefaultConfig({
    propFilter: { skipPropsWithoutDoc: true },
  }).parse,

  updateDocs(docs, file) {
    return {
      ...docs,
      props: docs.props.filter(p => {
        const isDefaultProp = isReactProp(p.name);
        return (
          // should be with description
          p.description &&
          // not default prop
          (!isDefaultProp ||
            // if default prop
            (isDefaultProp &&
              // should be with @override tag
              (p.tags.hasOwnProperty('override') ||
                // or should be listed at NO_FILTER_PROPS
                NO_FILTER_PROPS.includes(p.name))))
        );
      }),
    };
  },
};

interface IButtonProps extends React.HTMLAttributes<HTMLButtonElement> {
  /**
   * will NOT be displayed because listed in NO_FILTER_PROP
   */
  className?: string;
  /**
   * will be displayed because listed in NO_FILTER_PROP
   */
  width?: string;
  /**
   * will be displayed because not default prop
   */
  ripple?: boolean;
  /**
   * will be displayed because @override tag
   * @override
   */
  children?: React.ReactNode;
}

[OzairP]

Solved with

propFilter: props => props.parent && props.parent.fileName.startsWith('ui/src'),

[karolis-sh]

@OzairP this assumes the cloned repo directory name. You can improve this a bit:

    propFilter: props => {
      return (
        props.parent &&
        props.parent.fileName.startsWith(
          path
            .dirname(__filename)
            .split(path.sep)
            .pop() + '/src/'
        )
      );
    },

[geekftz]

i wonder if any annotations support to ignore property's api doc before output