-
Notifications
You must be signed in to change notification settings - Fork 4.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
Turbo Native Modules - Documentation ported from reactwg #4283
Conversation
✅ Deploy Preview for react-native ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Made to look similar to Github diffs
e3ce0c8
to
62c925f
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.
Great job and thanks for working on this. I left several comments, I believe we need to streamline the content and the language as much as possible.
docs/appendix.md
Outdated
|
||
## I. Terminology | ||
|
||
- **Schema** - TypeScript or Flow code that describes the API for a Turbo Native Module or Fabric Native component. Used by **Codegen** to generate boilerplate code. |
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.
😱 We always use Spec
for this definition, we never used Schema!
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 got mixed up. Just to confirm, the codegen pipeline is: Spec
→ Schema
→ Platform Code (protocols, interfaces, etc...)?
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.
yes, but the schema in json is an implementation details that the users should never see.
docs/turbo-native-modules.md
Outdated
<Tabs groupId="platforms" queryString defaultValue={constants.defaultPlatform}> | ||
<TabItem value="android" label="Android"> | ||
Codegen is executed through the `generateCodegenArtifactsFromSchema` Gradle task: | ||
|
||
```bash | ||
cd android | ||
./gradlew generateCodegenArtifactsFromSchema | ||
|
||
BUILD SUCCESSFUL in 837ms | ||
14 actionable tasks: 3 executed, 11 up-to-date | ||
``` | ||
|
||
This is automatically run when you build your Android application. | ||
</TabItem> | ||
<TabItem value="ios" label="iOS"> | ||
Codegen is run as part of the script phases that's automatically added to the project generated by Cocoapods. | ||
|
||
```bash | ||
cd ios | ||
bundle install | ||
bundle exec pod install | ||
bundle exec pod install | ||
|
||
... | ||
Framework build type is static library | ||
[Codegen] Adding script_phases to ReactCodegen. | ||
[Codegen] Generating ./build/generated/ios/ReactCodegen.podspec.json | ||
[Codegen] Analyzing /Users/me/src/TurboModuleExample/package.json | ||
[Codegen] Searching for codegen-enabled libraries in the app. | ||
[Codegen] Found TurboModuleExample | ||
[Codegen] Searching for codegen-enabled libraries in the project dependencies. | ||
[Codegen] Found react-native | ||
... | ||
``` | ||
|
||
</TabItem> | ||
</Tabs> |
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.
This can be removed from here. Those information are already in the codegen guide and we don't want that for an app developer to run codegen separatedly.
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.
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.
pod install
needs to be executed for sure, but I would remove the lines that refers to codegen, that's an implementation detail for the user.
I never used Android studio, but I can see the problem. The thing is that we would like for codegen to be transparent... If we ask the users to execute it manually, that's not transparent anymore. :(
cc. @cortinico what do you think about this?
Add mp4 + webm videos Add css tweaks
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.
Great job, left minor nits and some formatting.
docs/turbo-native-modules.md
Outdated
1. **define a typed JavaScript specification** using one of the most popular JavaScript type annotation languages: Flow or TypeScript; | ||
2. **configure your dependency management system to run Codegen**, which converts the specification into native language interfaces; | ||
3. **write you application code** using your specification | ||
4. **write your native platform code using the generated interfaces** to write and hook your native code into the React Native runtime environment |
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.
4. **write your native platform code using the generated interfaces** to write and hook your native code into the React Native runtime environment | |
4. **write your native platform code using the generated interfaces** to write and hook your native code into the React Native runtime environment. |
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.
@cipolleschi added all your feedback. I think we're good to go.
Turbo Native Modules Docs: