Skip to content
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

feat: Add gradle task to automatically copy the codegen artifacts for paper #2168

Conversation

maciekstosio
Copy link
Contributor

@maciekstosio maciekstosio commented Jun 3, 2024

Description

When changing native props on Fabric, codegen generates corresponding interfaces and delegates. To make sure both implementations are consistent, we implement those interfaces on Paper too. Currently, after generating interfaces using codegen, developer needs to copy corresponding files for paper manually. This task adds Gradle task, that automates this.

Changes

Add new task to build Gradle and necessary properties:

  • codegen artifacts dir and paper dir
  • flag in both fabric apps that indicates that copying should be performed (we do want this task to be performed only when developing the library)

Test code and steps to reproduce

Remove textColor from src/fabric/SearchBarNativeComponent.ts and run ./gradlew generateCodegenArtifactsFromSchema in /react-native-screens/FabricExample/android. That should automatically copy regenerated files to paper directory.

@maciekstosio maciekstosio requested a review from kkafar June 3, 2024 15:55
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

Left some remarks below

FabricExample/android/gradle.properties Show resolved Hide resolved
android/build.gradle Outdated Show resolved Hide resolved
android/build.gradle Outdated Show resolved Hide resolved
android/build.gradle Show resolved Hide resolved
android/build.gradle Outdated Show resolved Hide resolved
android/build.gradle Outdated Show resolved Hide resolved
android/build.gradle Outdated Show resolved Hide resolved
android/build.gradle Outdated Show resolved Hide resolved
android/build.gradle Outdated Show resolved Hide resolved
@maciekstosio maciekstosio requested a review from kkafar June 4, 2024 08:39
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answered you question, final notes & whitespaces.

android/build.gradle Outdated Show resolved Hide resolved
android/build.gradle Outdated Show resolved Hide resolved
android/build.gradle Outdated Show resolved Hide resolved
Example/android/gradle.properties Outdated Show resolved Hide resolved
TVOSExample/android/gradle.properties Outdated Show resolved Hide resolved
TestsExample/android/gradle.properties Outdated Show resolved Hide resolved
android/gradle.properties Outdated Show resolved Hide resolved
maciekstosio and others added 7 commits June 4, 2024 14:27
Co-authored-by: Kacper Kafara <[email protected]>
…en-artifacts' of github.com:software-mansion/react-native-screens into @maciekstosio/Add-gradle-task-to-automate-copying-codegen-artifacts
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some polishing and I think we're good to go

android/build.gradle Show resolved Hide resolved
android/gradle.properties Outdated Show resolved Hide resolved
android/gradle.properties Show resolved Hide resolved
android/gradle.properties Outdated Show resolved Hide resolved
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can proceed

@maciekstosio maciekstosio merged commit 5cfafbb into main Jun 4, 2024
8 checks passed
@maciekstosio maciekstosio deleted the @maciekstosio/Add-gradle-task-to-automate-copying-codegen-artifacts branch June 4, 2024 17:33
maciekstosio added a commit to software-mansion/react-native-gesture-handler that referenced this pull request Jun 10, 2024
## Description

Sibling of [similar task from
screens](software-mansion/react-native-screens#2168).
When changing native props on Fabric, codegen generates corresponding
interfaces and delegates. To make sure both implementations are
consistent, we implement those interfaces on Paper too. Currently, after
generating interfaces using codegen, developer needs to copy
corresponding files for paper manually. This task adds Gradle task, that
automates this.

## Changes

Add new task to build Gradle and necessary properties: 
- codegen artifacts dir and paper dir 
- flag in both fabric apps that indicates that copying should be
performed (we do want this task to be performed only when developing the
library)

## Test code and steps to reproduce

Remove `enabled` from
`src/specs/RNGestureHandlerButtonNativeComponent.ts` and run ` ./gradlew
generateCodegenArtifactsFromSchema` in `./FabricExample/android`. That
should automatically copy regenerated files to paper directory.

---------

Co-authored-by: Jakub Piasecki <[email protected]>
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
… paper (software-mansion#2168)

## Description

When changing native props on Fabric, codegen generates corresponding
interfaces and delegates. To make sure both implementations are
consistent, we implement those interfaces on Paper too. Currently, after
generating interfaces using codegen, developer needs to copy
corresponding files for paper manually. This task adds Gradle task, that
automates this.

## Changes

Add new task to build Gradle and necessary properties: 
- codegen artifacts dir and paper dir 
- flag in both fabric apps that indicates that copying should be
performed (we do want this task to be performed only when developing the
library)

## Test code and steps to reproduce

Remove `textColor` from `src/fabric/SearchBarNativeComponent.ts` and run
` ./gradlew generateCodegenArtifactsFromSchema` in
`/react-native-screens/FabricExample/android`. That should automatically
copy regenerated files to paper directory.

---------

Co-authored-by: Kacper Kafara <[email protected]>
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
…oftware-mansion#2224)

## Description

This task is rewrite to JS of
software-mansion#2168 with
some improvements. General point of creating those tasks is:

> When changing native props on Fabric, codegen generates corresponding
interfaces and delegates. To make sure both implementations are
consistent, we implement those interfaces on Paper too. Currently, after
generating interfaces using codegen, developer needs to copy
corresponding files for paper manually. This task adds Gradle task, that
automates this.

## Changes
Current assumption: 
Two scripts: `check-archs-consistency` and `sync-archs`. The first one
generates codegen interfaces and compares them with what we have for
paper, the second generates and copies for paper to sync the archs.
- sync is run pre build on example app
- sync is run when staged on changes to `src/paper`
- check is run on CI when `src/paper` or
`android/src/paper/java/com/facebook/react/viewmanagers` changes

What it improves:
- JS tasks are much faster, as codegen is JS script anyway, we skip
gradle and java setup all together (CI task down from 7min to 30s),
- we do not put code to library, so it shouldn't be possible to mess up
something for end users,
- instead of syncing archs when running codegen we do that on paper
example build and when staged, so: when developer didn't touch the code
it will have changes after commit, when developer switched to working on
paper interfaces should be always up to date when building the app.

## Test code and steps to reproduce

Open `src/fabric/ScreenStackHeaderConfigNativeComponent.ts` and remove
any proper form interface. Now:
- when building paper, interface should be updated
- when committing, interface should be updated
- if committed and pushed, Test consistency between Paper & Fabric
should fail :)
Brining back the prop and repeating up should cause the interface back
and CI green.
Posting changes in other places should cause CI task to run. 

You can also run those commands yourself using `yarn
check-archs-consistency` and `yarn sync-archs`

---------

Co-authored-by: Kacper Kafara <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants