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

Bugs and bugs around react-docgen #240

Closed
tadeuszwojcik opened this issue Aug 21, 2018 · 30 comments
Closed

Bugs and bugs around react-docgen #240

tadeuszwojcik opened this issue Aug 21, 2018 · 30 comments
Labels
bug Something isn't working difficult: high docz-core Issues related to package docz-core help wanted Extra attention is needed priority: high High priority issues

Comments

@tadeuszwojcik
Copy link

Question

How to pass custom config to react-docgen-typescript-loader ?
Description
Hi,
First of all, I love the project, much nicer experience than with styleguidist!

I'm trying to use it with typescript, which is working fine except PropsTable which renders empty.
I had similar issue when working with styleguidist, and solution for that was to pass my custom tsconfig.json to do react-docgen-typescript . Unfortunately I don't know how to do that with docz, I made changes locally inside docz code so react-docgen-typescript-loader reads my config and it works perfectly, but it's not a solution :) Is there a way to do that via modifyBundlerConfig ?
Thanks!

@tadeuszwojcik
Copy link
Author

I was able to apply custom config via docrc:

modifyBundlerConfig: config => {
    const jsxPluginIndex = config.plugins.findIndex(plugin => plugin.config.id === 'jsx');
    const { loaders } = config.plugins[jsxPluginIndex].config;
    const docGenLoaderIndex = loaders.findIndex(loader => /react-docgen-typescript-loader/.test(loader.loader));
    const docGenLoader = loaders[docGenLoaderIndex];

    docGenLoader.options = {
      tsconfigPath: './tsconfig.json'
    };

    return config;
  }

That's good, but unfortunately it's only working with react-docgen-typescript-loader 3.0.0-rc.0 as v2 does not parse options correctly .

Would you consider updating react-docgen-typescript-loader to ^3.0.0-rc.0 ? I think that would solve many issues for typescript users when props table renders empty (as they need to provide their own tsconfig not use default one from react-docgen-typescript)

@nicholasess nicholasess added the question Usage question or clarification request label Aug 24, 2018
@pedronauck pedronauck changed the title Passing custom config to react-docgen-typescript-loader Passing custom config to react-docgen Sep 11, 2018
@pedronauck pedronauck added bug Something isn't working and removed question Usage question or clarification request labels Sep 11, 2018
@pedronauck
Copy link
Member

pedronauck commented Sep 11, 2018

I chose to leave only this issue since it is related to the configuration of the react docgen. The idea is don't use plugins/loaders anymore and put all process related to docgen in the DataServer, with this in mind we can solve all issue marked here and get an awesome performance boost since we don't need to parse the entire code anymore.

@pedronauck pedronauck changed the title Passing custom config to react-docgen Bugs and bugs around react-docgen Sep 27, 2018
@pedronauck pedronauck added help wanted Extra attention is needed difficult: high priority: high High priority issues docz-core Issues related to package docz-core labels Sep 28, 2018
@itsdanielmatos
Copy link

I am also being affected by this issue when using an external library of react components (same issue as #253 @itReverie). Is there any quick solution available? 🙏

@JeroenReumkens
Copy link

@tadeuszwojcik Great find! By passing my own tsconfig I'm now getting the propstable rendered with the latest version of Docz as well!

@pedronauck Might be a 'simple' fix now to get this working again? Seems like the quick solutions for now would be to make an easy option to pass a custom config to React Docgen (instead of the 5 lines of ugly JS in the config), and we're good to go?

@lwxyfer
Copy link

lwxyfer commented Oct 21, 2018

@JeroenReumkens react-docgen-typescript-loader has released v3.0.0.

when I change

export class Button

to

class Button
export default Button

It works as described in the document

@IssueHuntBot
Copy link

@BoostIO funded this issue with $40. See it on IssueHunt

@mattfelten
Copy link

I'm looking for this feature as well. I'm retrofitting Docz into an existing TS project (hoping to merge our old docs site and Styleguidist together) and PropsTable is showing up empty. Definitely not able to migrate yet. Hopefully soon!

@JeroenReumkens
Copy link

The solution given above seems to be broken in the latest release of Docz. @pedronauck Do you remember changing anything related to the way docgen is added to the webpack config?

@guilhermedecampo
Copy link

guilhermedecampo commented Dec 19, 2018

Hey guys I had the same problem with the react-docgen-typescript-loader passing all the props of react on docz because I'm using styled-components. I searched a bit and found a config for this. And because docz is extensible as f I could update the bundler config. Top @pedronauck 🚀 .

  modifyBundlerConfig: config => ({
    ...config,
    module: {
      ...config.module,
      rules: config.module.rules.map(rule => ({
        ...rule,
        use: rule.use.map(ruleUse =>
          ruleUse.loader.includes('react-docgen-typescript-loader')
            ? {
                ...ruleUse,
                options: {
                  propFilter: prop => {
                    if (prop.parent == null) return true
                    return !prop.parent.fileName.includes('node_modules')
                  },
                },
              }
            : ruleUse,
        ),
      })),
    },
  }),

@pedronauck
Copy link
Member

Hey guys, I put the @guilhermedecampo solution inside docz, see if is good now 🙏

@JeroenReumkens
Copy link

Hey @pedronauck You mean Docz should do this by itself now? If so, can you tell me in what version I can find the fix?

The solution given above doesn't look that easy a first glance. So if that is the way forward, I'll try to implement it in my project to see if it works. But if it does work, I'll try to make the above code a bit more simple / readable.

@IssueHuntBot
Copy link

@pedronauck has submitted a pull request. See it on IssueHunt

@IssueHuntBot
Copy link

@pedronauck has rewarded $28.00 to @pedronauck. See it on IssueHunt

  • 💰 Total deposit: $40.00
  • 🎉 Repository reward(20%): $8.00
  • 🔧 Service fee(10%): $4.00

@pedronauck
Copy link
Member

In the next release (v0.14) I'll introduce a new way to parse props using react-docgen. Instead of using a babel plugin for prop-types and a loader for typescript, everything now is centralized in a process using data server:

https://github.com/pedronauck/docz/blob/v0.14/core/docz-core/src/states/props.ts

With these changes, I think that we will improve a lot the performance since all props extracting logic will not belong to webpack anymore.

I'll finish the v0.14 that will have some other improvements, then I came back here to test and close this issue 🙏

@ondrejbartas
Copy link

@pedronauck Hi when do you think you will be releasing v0.14?

@mgallagher
Copy link

Pretty sure these updates are coming in v1! #656

New docgen parse using DataServer instead of babel plugin and loader (85499a8)

@glockjt
Copy link

glockjt commented Mar 22, 2019

Is this in v1? I just tried out v1 and Props are still not working with Styled Components.

@rfoel
Copy link
Contributor

rfoel commented Mar 24, 2019

@glockjt Check if this line helps you.

Instead of exporting the styled-component directly, try to make a new component using the styled-component. And declare the prop types in the new component like the example.

@rfoel
Copy link
Contributor

rfoel commented Mar 24, 2019

Can the Props component accept something besides a static prop-types object? I have a function that generates props types for my component, that I reuse across many components, and if it's not a exact prop-types object it won't render in the Props component.

@glockjt
Copy link

glockjt commented Mar 25, 2019

@rfoel that doesn't work either. Even if it did, this is still an issue, you shouldn't have to change the way you create your styled components just for documentation to work.

@rfoel
Copy link
Contributor

rfoel commented Mar 27, 2019

@glockjt That I agree. I'm having a hard time trying to make the props work.

@100ideas
Copy link

Ran into similar issues.

Trying to get docz@1.0.0-rc.3 working with remirror. It's a react library for prosemirror:

  • written mostly in typescript
  • src is documented liberally with jsdoc comments
  • with multiple packages in a 'yarn workspace + lerna' monorepo
  • that does work with the previous version of docz thanks to some significant docz babel + webpack configuration.

I kept a bunch of notes on my brute-force search of webpack/babel configuration space for docz@1, since I am webpack-illiterate: https://github.com/100ideas/remirror/blob/doczv1/support/docz-v1-WIP-notes/NOTES-docz-v1.0.0-experiments.md

will update if we make any progress.

What's hard is figuring out how to get docz working not just with example.mdx demonstration docs, but also set up somehow to provide API-browsing like experience of the source itself based on all the jsdoc comments.

@pedronauck
Copy link
Member

I'm releasing some new release candidate today @100ideas, one of your problems I think that is fixed!

@100ideas
Copy link

woot rc5! will try hopefully later today.

@mikeplis
Copy link

mikeplis commented Apr 4, 2019

Are there plans to allow the Props component to work with prop types that are imported from another file and spread into a component's propTypes? I just tried on 1.0.0-rc.7 and it didn't work.

// utils.js
export const color = () => {
    // ...
}

color.propTypes = {
    color: PropTypes.oneOfType([
        PropTypes.string,
        PropTypes.arrayOf(PropTypes.string)
    ]),
};

// Button.js
import React from 'react';
import PropTypes from 'prop-types';

import { color } from '../path/to/utils';

export const Button = props => {
    // ...
};

Button.propTypes = {
    ...color.propTypes, // <- it would be awesome if this showed up in the props table
};

export default Button;

It seems like there was some attempt to get it working here?

@rfoel
Copy link
Contributor

rfoel commented Apr 6, 2019

@pedronauck do you have an update about the comment above? Working on this would help a lot.

@pedronauck
Copy link
Member

This handler should fix this issue @rfoel, but since the newest version of react-docgen it doesn't work anymore, need to be updated, but I think that the owner of the repo dropped it out of the project.

@pedronauck
Copy link
Member

I'll close this issue since most cases were fixed.

@mikeplis
Copy link

mikeplis commented Apr 9, 2019

@pedronauck I agree that the handler you linked is pretty outdated, but it seems like other, newer packages have tried to solve the same problem, like react-docgen-imported-proptype-handler which is already referenced in the docz code here, but commented out.

Was there an issue when you tried using it before?

@shermanhui
Copy link

@pedronauck is there any update on this potential solution or how to get react-docgen-typescript-loader to not ignore props being imported from packages?

I'm having a similar issue in our TS monorepo where the props being imported from another component or node_modules package is not being read by react-docgen-typescript-loader.

Does the changes to DataServer address this (if so, how do I configure docz properly to accept props imported from node_modules) or would it still be acceptable to apply a custom config to docGenLoader via modifyBundleConfig?

@pedronauck I agree that the handler you linked is pretty outdated, but it seems like other, newer packages have tried to solve the same problem, like react-docgen-imported-proptype-handler which is already referenced in the docz code here, but commented out.

Was there an issue when you tried using it before?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working difficult: high docz-core Issues related to package docz-core help wanted Extra attention is needed priority: high High priority issues
Projects
None yet
Development

No branches or pull requests