chore: change gradle task for copying to new archs into JS scripts #2224

maciekstosio · 2024-07-02T11:53:05Z

Description

This task is rewrite to JS of #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

kkafar

Just initial remarks - I'll have more once the PR is opened & ready for review. The approach is good.

.github/workflows/check-paper-integrity.yml

package.json

Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>

…nges to specififc path/branch

scripts/codegen-check-consistency.js

scripts/codegen-utils.js

WoLewicki

Added 2 comments

scripts/codegen-utils.js

WoLewicki · 2024-07-08T16:12:26Z

Example/android/app/build.gradle

@@ -135,3 +135,13 @@ dependencies {
 }

 apply from: file("../../node_modules/@react-native-community/cli-platform-android/native_modules.gradle"); applyNativeModulesAppBuildGradle(project)
+apply plugin: 'com.github.node-gradle.node'
+
+task syncArchs(type: NodeTask) {


I don't get it why it's still here. Don't we do everything on the JS side now?

It's in the app so it shouldn't interfere with the lib. I understood that @kkafar wanted to have it run automatically while developing. It needs to be added per example/test app, so I can just not add it in other packages.

Yeah & I want this in screens.

j-piasecki

I don't think I have anything else to add here 😄

kkafar

Looks good now, thanks 👍🏻

kkafar · 2024-07-09T12:38:52Z

Example/android/app/build.gradle

@@ -135,3 +135,13 @@ dependencies {
 }

 apply from: file("../../node_modules/@react-native-community/cli-platform-android/native_modules.gradle"); applyNativeModulesAppBuildGradle(project)
+apply plugin: 'com.github.node-gradle.node'
+
+task syncArchs(type: NodeTask) {


Yeah & I want this in screens.

scripts/codegen-utils.js

tboba

Just a one suggestion from me. Great change!

tboba · 2024-07-09T14:12:55Z

.github/workflows/check-archs-consistency.yml

+        uses: actions/checkout@v2
+      - name: Use Node.js 18
+        uses: actions/setup-node@v2


Let's use v4 script versions.

Suggested change

uses: actions/checkout@v2

- name: Use Node.js 18

uses: actions/setup-node@v2

uses: actions/checkout@v4

- name: Use Node.js 18

uses: actions/setup-node@v4

tboba · 2024-07-09T14:16:23Z

scripts/codegen-utils.js

Great job with this one!

## Description Fixes to #2224, that came out in other repos. ## Changes - Bump version of GitHub actions - Fix warning when trying to copy non existing codegen interface ## Test code and steps to reproduce Duplicate `android/src/paper/java/com/facebook/react/viewmanagers/RNSSearchBarManagerDelegate.java` with name change for example with ` copy` at the end. Before it would fail because file wouldn't be found. Now there will be warning in logs.

…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 <kacper.kafara@swmansion.com>

## Description Fixes to software-mansion#2224, that came out in other repos. ## Changes - Bump version of GitHub actions - Fix warning when trying to copy non existing codegen interface ## Test code and steps to reproduce Duplicate `android/src/paper/java/com/facebook/react/viewmanagers/RNSSearchBarManagerDelegate.java` with name change for example with ` copy` at the end. Before it would fail because file wouldn't be found. Now there will be warning in logs.

maciekstosio added 6 commits July 2, 2024 13:52

chore: change gradle task for copying to new archs into JS scripts

4eaf9fe

fix: add installing dependencies

a4a707a

chore: run prettier on new scripts and fix scripts name

b214415

fix: take react-native from root node modules

5d250e2

chore: test

bbace06

chore: revert test

51f3397

kkafar reviewed Jul 3, 2024

View reviewed changes

.github/workflows/check-paper-integrity.yml Outdated Show resolved Hide resolved

.github/workflows/check-paper-integrity.yml Outdated Show resolved Hide resolved

.github/workflows/check-paper-integrity.yml Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

maciekstosio and others added 5 commits July 8, 2024 14:27

feat: run sync archs on prebuild

bfbc266

Update .github/workflows/check-paper-integrity.yml

cd5e7d0

Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>

Update .github/workflows/check-paper-integrity.yml

54bcd79

Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>

chore: fix task name, update name and run github workflow only on cha…

8e4f7a0

…nges to specififc path/branch

chore: test pipeline

2b31454

maciekstosio mentioned this pull request Jul 8, 2024

fix: correct misspelings in gradle task for automate copy #2172

Closed

maciekstosio added 2 commits July 8, 2024 15:22

chore: test pipeline 2

adb798d

chore: roll back changes for test

4106011

maciekstosio marked this pull request as ready for review July 8, 2024 13:26

maciekstosio added 2 commits July 8, 2024 15:30

chore: fix line end

6a24cee

chore: fix end of file with empty line

bdf4f93

j-piasecki reviewed Jul 8, 2024

View reviewed changes

scripts/codegen-check-consistency.js Outdated Show resolved Hide resolved

scripts/codegen-utils.js Show resolved Hide resolved

scripts/codegen-utils.js Outdated Show resolved Hide resolved

chore: add changes from comments

1b2a000

WoLewicki reviewed Jul 8, 2024

View reviewed changes

maciekstosio requested review from j-piasecki and kkafar July 9, 2024 07:20

j-piasecki approved these changes Jul 9, 2024

View reviewed changes

feat: extract package name prefix to the variable

3e8dc1e

maciekstosio requested a review from WoLewicki July 9, 2024 07:50

kkafar approved these changes Jul 9, 2024

View reviewed changes

Merge branch 'main' into @maciekstosio/Change-gradle-copy-task-to-JS-

cfaa6da

tboba self-requested a review July 9, 2024 14:12

tboba approved these changes Jul 9, 2024

View reviewed changes

WesleyR10 mentioned this pull request Aug 21, 2024

[Snyk] Upgrade react-native-screens from 3.31.1 to 3.33.0 WesleyR10/IgniteTeams#5

Open

shonore mentioned this pull request Aug 22, 2024

[Snyk] Upgrade react-native-screens from 3.0.0 to 3.33.0 shonore/Fickle#1609

Open

2lambda123 mentioned this pull request Aug 22, 2024

[Snyk] Upgrade react-native-screens from 3.32.0 to 3.33.0 2lambda123/abp#163

Merged

Cabreira97 mentioned this pull request Aug 23, 2024

[Snyk] Upgrade react-native-screens from 3.31.1 to 3.33.0 Cabreira97/Connect-Tech#5

Closed

2lambda123 mentioned this pull request Aug 26, 2024

[Snyk] Upgrade react-native-screens from 3.33.0 to 3.34.0 2lambda123/abp#164

Merged

shonore mentioned this pull request Sep 3, 2024

[Snyk] Upgrade react-native-screens from 3.0.0 to 3.34.0 shonore/Fickle#1669

Open

undrthegraveyard mentioned this pull request Sep 4, 2024

[Snyk] Upgrade react-native-screens from 3.31.1 to 3.34.0 undrthegraveyard/gigslick#5

Closed

asraful-cse mentioned this pull request Sep 6, 2024

[Snyk] Upgrade react-native-screens from 3.11.1 to 3.34.0 asraful-cse/planet_app_react_native#412

Open

Ayush-Bulbule mentioned this pull request Oct 2, 2024

[Snyk] Upgrade react-native-screens from 3.22.1 to 3.34.0 Ayush-Bulbule/FoodleApp#5

Merged

tobistudio mentioned this pull request Oct 3, 2024

[Snyk] Upgrade react-native-screens from 3.27.0 to 3.34.0 tobistudio/React_Native_Audio#75

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: change gradle task for copying to new archs into JS scripts #2224

chore: change gradle task for copying to new archs into JS scripts #2224

maciekstosio commented Jul 2, 2024 •

edited

Loading

kkafar left a comment

WoLewicki left a comment

WoLewicki Jul 8, 2024

maciekstosio Jul 9, 2024

kkafar Jul 9, 2024

j-piasecki left a comment

kkafar left a comment

kkafar Jul 9, 2024

tboba left a comment

tboba Jul 9, 2024

tboba Jul 10, 2024

tboba Jul 9, 2024

chore: change gradle task for copying to new archs into JS scripts #2224

chore: change gradle task for copying to new archs into JS scripts #2224

Conversation

maciekstosio commented Jul 2, 2024 • edited Loading

Description

Changes

Test code and steps to reproduce

kkafar left a comment

Choose a reason for hiding this comment

WoLewicki left a comment

Choose a reason for hiding this comment

WoLewicki Jul 8, 2024

Choose a reason for hiding this comment

maciekstosio Jul 9, 2024

Choose a reason for hiding this comment

kkafar Jul 9, 2024

Choose a reason for hiding this comment

j-piasecki left a comment

Choose a reason for hiding this comment

kkafar left a comment

Choose a reason for hiding this comment

kkafar Jul 9, 2024

Choose a reason for hiding this comment

tboba left a comment

Choose a reason for hiding this comment

tboba Jul 9, 2024

Choose a reason for hiding this comment

tboba Jul 10, 2024

Choose a reason for hiding this comment

tboba Jul 9, 2024

Choose a reason for hiding this comment

maciekstosio commented Jul 2, 2024 •

edited

Loading