-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Refactor codegen: Dispatch props and events from a central place. #34975
Conversation
Base commit: e4b5d3e |
Base commit: e4b5d3e |
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.
Hi, thank for taking this.
I left a couple of comments in the PR.
Plus, are you going to make the same changes for Flow?
I think we should strive to keep the two languages aligned, so it will be easier to create a common denominator to reduce the code duplication (as we are doing for the modules)
packages/react-native-codegen/src/parsers/typescript/components/events.js
Outdated
Show resolved
Hide resolved
packages/react-native-codegen/src/parsers/typescript/components/props.js
Outdated
Show resolved
Hide resolved
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 added some more comments/refactoring to improve typings and readability.
packages/react-native-codegen/src/parsers/typescript/components/extends.js
Outdated
Show resolved
Hide resolved
packages/react-native-codegen/src/parsers/typescript/components/extends.js
Outdated
Show resolved
Hide resolved
packages/react-native-codegen/src/parsers/typescript/components/extends.js
Outdated
Show resolved
Hide resolved
19f0700
to
b448c6e
Compare
b448c6e
to
f33ed6f
Compare
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.
Thanks for these simplifications. There are a couple of linting problems due to unused variables, but it is otherwise good.
Do you think we can do something similar for Flow as well, to keep the two aligned?
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This pull request was successfully merged by @ZihanChen-MSFT in affcfa7. When will my fix make it into a release? | Upcoming Releases |
…cebook#34975) Summary: Refer to `In component, props and events pick properties from the component interface by themselves independently, I would like to reverse the direction, to have a function dispatching properties to props and events, to reduce duplicated code. ` in facebook#34872 ## Changelog [General] [Changed] - Refactor codegen: Dispatch props and events from a central place. Pull Request resolved: facebook#34975 Test Plan: `yarn jest react-native-codegen` passed Reviewed By: cortinico Differential Revision: D40507917 Pulled By: cipolleschi fbshipit-source-id: 71988877008ae11a01affd31b34bb3a3b88f96be
Summary
Refer to
In component, props and events pick properties from the component interface by themselves independently, I would like to reverse the direction, to have a function dispatching properties to props and events, to reduce duplicated code.
in #34872Changelog
[General] [Changed] - Refactor codegen: Dispatch props and events from a central place.
Test Plan
yarn jest react-native-codegen
passed