-
-
Notifications
You must be signed in to change notification settings - Fork 848
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
Convert camera and update doc generation #2057
Conversation
content = content.replace(/memo\(forwardRef\((.+?)\)\)/, '$1'); | ||
content = content.replace(/useCallback\(([^,]+), [^)]+\)/g, '$1'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think those can now be removed with 6.0.0-alpha.3
of react-docgen
package.json
Outdated
@@ -77,7 +78,7 @@ | |||
"node-dir": "0.1.17", | |||
"prettier": "2.6.2", | |||
"react": "17.0.2", | |||
"react-docgen": "^5.0.0-beta.1", | |||
"react-docgen": "rnmapbox/react-docgen#rnmapbox", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"react-docgen": "rnmapbox/react-docgen#rnmapbox", | |
"react-docgen": "6.0.0-alpha.3", |
I think it now contains the fixes we need See 592
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if I integrate this, imported types (e.g. AtmosphereLayerStyleProps
in Atmosphere.tsx
) still don't expand in the documentation, and methods on the ref don't appear in component.methods
anymore. Any idea how you want to handle this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized this is still open: reactjs/react-docgen#591. Would you want to rebase and we point to your fork? Or I would be happy to set up a patch.
Would really like to just get this merged 😭 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the PR and also pushed the merged version to: https://github.com/rnmapbox/react-docgen/tree/basic-hook-support. But last time I was checking it did not generate methods, because our structure is different to what's supported, but worth a try.
@naftalibeder test are failing, otherwise it's looks good to go for me |
Everything should be good. I tried pointing to the fork of Feel free to merge / let me know if I can! |
Thanks much, for the hard work and patience! |
Haha thanks for sticking with this to the end! |
Description
This converts the camera to functional component written in Typescript, and updates doc generation to correctly read props in a component like this.
I tried to keep the changes as minimal as possible, although this means that the
Atmosphere
documentation is overwritten with a less useful version. This is because of the larger issue with the generator, where imported types do not get expanded, but local types in the component file do.Checklist
yarn generate
in the root folderCHANGELOG.md
index.d.ts
)/example
)