NunoPinheiro / vaadin-react

7 stars 3 forks source link

Too many limitions by react-docgen #1

Open LaiZhou opened 8 years ago

LaiZhou commented 8 years ago

Too many limitions by react-docgen ,I tried many components ,such as http://react-bootstrap.github.io/, it throws error.May have better way to parse react components?


/Users/zhoulai/.nvm/versions/node/v4.2.1/lib/node_modules/vaadin-react/node_modules/react-docgen/dist/parse.js:78
  throw new Error(ERROR_MISSING_DEFINITION);
  ^

Error: No suitable component definition found.
    at parse (/Users/zhoulai/.nvm/versions/node/v4.2.1/lib/node_modules/vaadin-react/node_modules/react-docgen/dist/parse.js:78:9)
    at Object.defaultParse [as parse] (/Users/zhoulai/.nvm/versions/node/v4.2.1/lib/node_modules/vaadin-react/node_modules/react-docgen/dist/main.js:66:30)
    at processFile (/Users/zhoulai/.nvm/versions/node/v4.2.1/lib/node_modules/vaadin-react/generator/generator.js:43:33)
    at Object.<anonymous> (/Users/zhoulai/.nvm/versions/node/v4.2.1/lib/node_modules/vaadin-react/generator/generator.js:37:3)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Function.Module.runMain (module.js:467:10)
    at startup (node.js:134:18)

···
NunoPinheiro commented 8 years ago

Hi, yes using react-docgen is a big limitation. I am not fond of parsers for this use case.

My first approach was to load the components (using require) but react proptypes are functions and we can't extrapolate information in runtime, so I fell back to using react docgen.

I have now dedicated an hour to check if I can somehow return to my first approach. I will be overriding the react proptypes with a runtime readable version so any component can be loaded and the information can be extracted.

I will comment in here again when I have a first working version.

NunoPinheiro commented 8 years ago

Hi, I removed the react docgen and started the approach I reffered on the previous comment. Please check out the project and run the script directly to use the latest version.

I tried using react-bootstrap. The parsing part is now working, but it seems that most components have "oneOf" properties, which are still unsupported by this project.

moimael commented 7 years ago

I use your reactReflectiveProptypes.js and I must say it's amazing, almost exactly what I needed.

A few remark though, required is always set to true, even if isRequired is not specified in propTypes. Also any reason to make an isRequired object with an empty function and have type and required inside ?

Also line 9 and 30 are never used.

Thanks awesome work :)

NunoPinheiro commented 7 years ago

Hi @moimael , thanks for the comment.

I am going to check the required issue. The reason to make an empty isRequired object is to ensure that when you declare your properties using "isRequired" it won't return undefined, it should return a the same property but with required setted as true

NunoPinheiro commented 7 years ago

Hi, the required field is only set to true when using the "isRequired" field. Unless I am missing some specific corner case. When you do not use the isRequired, the field is not set, so required is 'undefined'.

I've also checked the lines 9 and 30, I'll be removing them.

Thank you for the feedback. If you have other questions or would like any new feature please tell me