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

bit autodocs should support non-primitive types #2487

Closed
KutnerUri opened this issue Mar 24, 2020 · 4 comments · Fixed by #2517
Closed

bit autodocs should support non-primitive types #2487

KutnerUri opened this issue Mar 24, 2020 · 4 comments · Fixed by #2517
Assignees
Labels

Comments

@KutnerUri
Copy link
Contributor

Bit should detect prop 'elevation' for this component:

import React from 'react';
import classNames from 'classnames';
import styles from './card.module.scss';
import elevations from './elevations.module.scss';

export type CardProps = {
	/**
	 * Controls the shadow cast by the card, to generate a "stacking" effects.
	 * For example, a modal floating over elements may have a 'high' elevation
	 */
	elevation: 'none' | 'low' | 'medium' | 'high';
} & React.HTMLAttributes<HTMLDivElement>;

/**
 * A wrapper resembling a physical card, grouping elements and improve readability.
 */
export function Card({ className, elevation, ...rest }: CardProps) {
	return (
		<div className={classNames(styles.card, elevations[elevation], className)} {...rest} />
	);
}

Card.defaultProps = {
	elevation: 'low',
};

I expect to get these docs:

  ┌────────────────────┬──────────────────────────────────────────────────┐
  │ Name               │ Card                                             │
  ├────────────────────┼──────────────────────────────────────────────────┤
  │ Description        │ A wrapper resembling a physical card, grouping   │
  │                    │ elements and improve readability.                │
  ├────────────────────┼──────────────────────────────────────────────────┤
  │ Properties         │ (elevation: 'none' | 'low' | 'medium' | 'high')  │
  └────────────────────┴──────────────────────────────────────────────────┘

Actually, I get these docs:

  ┌────────────────────┬──────────────────────────────────────────────────┐
  │ Name               │                                                  │
  ├────────────────────┼──────────────────────────────────────────────────┤
  │ Description        │ Controls the shadow cast by the card, to         │
  │                    │ generate a "stacking" effects.                   │
  │                    │ For example, a modal floating over elements may  │
  │                    │ have a 'high' elevation                          │
  └────────────────────┴──────────────────────────────────────────────────┘
  ┌────────────────────┬──────────────────────────────────────────────────┐
  │ Name               │                                                  │
  ├────────────────────┼──────────────────────────────────────────────────┤
  │ Description        │ A wrapper resembling a physical card, grouping   │
  │                    │ elements and improve readability.                │
  └────────────────────┴──────────────────────────────────────────────────┘

Specifications

  • Bit version: 14.7.6
@KutnerUri KutnerUri added type/bug area/docs-parsing automated docs parsing in bit labels Mar 24, 2020
@GiladShoham
Copy link
Member

GiladShoham commented Mar 30, 2020

We are using react-docgen to generate react docs.
In your example, it looks like the react-docgen failed to parse it, so we fall back to some regex strategy.
Looking in their playground
I got this value

{
  "description": "A wrapper resembling a physical card, grouping elements and improve readability.",
  "displayName": "Card",
  "methods": [],
  "props": {
    "elevation": {
      "required": false,
      "tsType": {
        "name": "union",
        "raw": "'none' | 'low' | 'medium' | 'high'",
        "elements": [
          {
            "name": "literal",
            "value": "'none'"
          },
          {
            "name": "literal",
            "value": "'low'"
          },
          {
            "name": "literal",
            "value": "'medium'"
          },
          {
            "name": "literal",
            "value": "'high'"
          }
        ]
      },
      "description": "Controls the shadow cast by the card, to generate a \"stacking\" effects.\nFor example, a modal floating over elements may have a 'high' elevation",
      "defaultValue": {
        "value": "'low'",
        "computed": false
      }
    }
  }
}

Looks like the data is there, so it should be pretty easy to support it.
(we use version 4.1.1 while latest is 5.3.0, so it might require an upgrade first)

@davidfirst davidfirst self-assigned this Mar 30, 2020
GiladShoham added a commit that referenced this issue Mar 31, 2020
* upgrade react-docgen from 4.x to 5.x.

* resolve #2487, fix react docs of union type prop

* update pkg version

* disable pack temporary because of vercel/pkg#883

Co-authored-by: Gilad Shoham <shoham.gilad@gmail.com>
@KutnerUri
Copy link
Contributor Author

KutnerUri commented Mar 31, 2020

hey, it's working nicely. I did get an unexpected rewire on export, talking to Gilad about it.

Still doesn't work for this component:
https://bit.dev/bit/base/atoms/ref-tooltip


Documentation

  ┌────────────────────┬──────────────────────────────────────────────────┐
  │ Name               │                                                  │
  ├────────────────────┼──────────────────────────────────────────────────┤
  │ Description        │ dom element to attach to                         │
  └────────────────────┴──────────────────────────────────────────────────┘
  ┌────────────────────┬──────────────────────────────────────────────────┐
  │ Name               │                                                  │
  ├────────────────────┼──────────────────────────────────────────────────┤
  │ Description        │ options for the underlying Popper.js position    │
  │                    │ engine                                           │
  └────────────────────┴──────────────────────────────────────────────────┘
  ┌────────────────────┬──────────────────────────────────────────────────┐
  │ Name               │                                                  │
  ├────────────────────┼──────────────────────────────────────────────────┤
  │ Description        │ A [Popper.js](https://popper.js.org/) react      │
  │                    │ wrapper that repositions children to be adjacent │
  │                    │ to a target element.                             │
  │                    │ This component is a container only, with no      │
  │                    │ visual styling.                                  │
  └────────────────────┴──────────────────────────────────────────────────┘

@davidfirst
Copy link
Member

@KutnerUri , is this still an issue?
If so, the errors coming from the react-docs parser are of level "silly", which by default are not shown in the log file.
To get them, please run bit config set log_level silly.
Once you get the error, please post it here.

@KutnerUri
Copy link
Contributor Author

oh I will try this config.

I can confirm this is solved in the dev version of bit, plus some removal of unsupported syntax, like private.
You can see the fixed component here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@davidfirst @GiladShoham @KutnerUri and others