reactjs / react-docgen

A CLI and library to extract information from React component files for documentation generation purposes.
https://react-docgen.dev
MIT License
3.61k stars 291 forks source link

No suitable component definition found for higher-order component #288

Open emccorson opened 5 years ago

emccorson commented 5 years ago

react-docgen can't find a component definition when the component is defined as a higher-order component:

import React from 'react';

const wrap = (CustomElement) => (props) =>
    <CustomElement { ...props } >{ props.children }</CustomElement>

const Test = wrap('custom-element');

export default Test;
> node_modules/.bin/react-docgen src/components/Test.jsx

Error with path "src/components/Test.jsx": Error: No suitable component definition found.
Error: No suitable component definition found.
    at parse (/private/tmp/test/bindings/react/node_modules/react-docgen/dist/parse.js:84:9)
    at Object.defaultParse [as parse] (/private/tmp/test/bindings/react/node_modules/react-docgen/dist/main.js:66:30)
    at parse (/private/tmp/test/bindings/react/node_modules/react-docgen/bin/react-docgen.js:103:17)
    at /private/tmp/test/bindings/react/node_modules/react-docgen/bin/react-docgen.js:204:30
    at FSReqWrap.oncomplete (fs.js:153:5)

If I apply the function myself, react-docgen works as expected:

const Test = (props) =>
    <custom-element { ...props } >{ props.children }</custom-element>
> node_modules/.bin/react-docgen src/components/Test.jsx

{"description":"","displayName":"Test","methods":[]}

Tested with react-docgen@2.21.0.

fkling commented 5 years ago

Yeah, react-docgen doesn't detect HOCs themselves, it would only be able to detect the component you pass to the HOC. But since you are not passing an actual component, it finds nothing.

Am I right in assuming that you would want react-docgen to pickup the component returned by wrap?

I think sometimes people want documentation for the component passed to the HOC, sometimes they want documentation for the HOC itself.

Not sure how to solve this problem but worth revisiting.

trevordmiller commented 5 years ago

It would be helpful for "react-docgen to pickup the component returned by wrap" IMO

neilff commented 5 years ago

I've noticed similar behavior with components which only fire hooks, for example:

const FireHook = ({ children }) => {
  useEffect(() => {
    console.log('fire');
  });

  return children;
}

react-docgen fails to pickup the definition for this component. As a work around for the above, I found wrapping children in React.Fragment will allow it to be parsed by react-docgen.

fkling commented 5 years ago

@neilff, yeah, that's basically the same issue as https://github.com/reactjs/react-docgen/issues/336 .

emccorson commented 2 years ago

I managed to find a workaround that works for my components at least by hacking at the findExportedComponentDefinition resolver so it just returns whatever is exported without trying to resolve it to a component.

It works with my component in the form:

const AlertDialogButton = onsCustomElement('ons-alert-dialog-button');
AlertDialogButton.propTypes = { /* ... */ };
export default AlertDialogButton;

To use it, apply the patch to https://github.com/reactjs/react-docgen/blob/v5.4.0/src/resolver/findExportedComponentDefinition.js

diff --git a/src/resolver/findExportedComponentDefinition.js b/src/resolver/findExportedComponentDefinition.js
index 0ad8b00..7d88aba 100644
--- a/src/resolver/findExportedComponentDefinition.js
+++ b/src/resolver/findExportedComponentDefinition.js
@@ -76,14 +76,7 @@ export default function findExportedComponentDefinition(
   function exportDeclaration(path) {
     const definitions = resolveExportDeclaration(path).reduce(
       (acc, definition) => {
-        if (isComponentDefinition(definition)) {
-          acc.push(definition);
-        } else {
-          const resolved = resolveToValue(resolveHOC(definition));
-          if (isComponentDefinition(resolved)) {
-            acc.push(resolved);
-          }
-        }
+        acc.push(definition);
         return acc;
       },
       [],
@@ -96,7 +89,7 @@ export default function findExportedComponentDefinition(
       // If a file exports multiple components, ... complain!
       throw new Error(ERROR_MULTIPLE_DEFINITIONS);
     }
-    foundDefinition = resolveDefinition(definitions[0]);
+    foundDefinition = definitions[0];
     return false;
   }