[WIP] feat(crnl): support nitro modules #721

atlj · 2024-12-06T17:11:31Z

WIP until #732 is merged

Summary

Adds Nitro Modules support to create-react-native-library.

What's next?

Test plan

Prior to all cases:

Clone this branch
Call yarn and yarn prepare
Call bin/create-react-native-library

Example app

Create a new nitro module
Go to the module and install node dependencies
Call yarn nitrogen to create nitro bindings
Build the Android app and make sure it works properly
Install the pods and build the iOS app and make sure it works properly

Published lib

Publish the library from the previous case using https://github.com/wclr/yalc (yalc publish)
Create a new React Native app
Call yalc add your-lib
Make sure the React Native app builds and works fine with your lib.

mrousavy · 2024-12-06T17:14:56Z

letsgoooooo!!!

mrousavy

really excited for this!

.github/workflows/build-templates.yml

packages/create-react-native-library/src/exampleApp/generateExampleApp.ts

packages/create-react-native-library/src/inform.ts

mrousavy · 2024-12-06T17:17:25Z

packages/create-react-native-library/templates/common/$package.json

@@ -85,10 +91,16 @@
    "eslint-config-prettier": "^9.0.0",
    "eslint-plugin-prettier": "^5.0.1",
    "jest": "^29.7.0",
+<% if (project.nitro) { -%>
+    "nitro-codegen": "^<%- versions.nitroCodegen %>",


ah here we have it - ok good!

packages/create-react-native-library/templates/common/CONTRIBUTING.md

packages/create-react-native-library/templates/native-common/android/build.gradle

mrousavy · 2024-12-06T17:19:28Z

packages/create-react-native-library/templates/nitro-module/ios/{%- project.name %}.swift

+    public var hybridContext = margelo.nitro.HybridContext()
+    public var memorySize: Int {
+        getSizeOf(self)
+    }


I should really work on getting this lifted into the spec instead of having it at user class level. Maybe Nitro 19

rodgomesc · 2024-12-06T17:44:52Z

packages/create-react-native-library/templates/native-common/android/build.gradle

+
+  packagingOptions {
+    excludes = [
+            "META-INF",
+            "META-INF/**",
+            "**/libc++_shared.so",
+            "**/libfbjni.so",
+            "**/libjsi.so",
+            "**/libfolly_json.so",
+            "**/libfolly_runtime.so",
+            "**/libglog.so",
+            "**/libhermes.so",
+            "**/libhermes-executor-debug.so",
+            "**/libhermes_executor.so",
+            "**/libreactnative.so",
+            "**/libreactnativejni.so",
+            "**/libturbomodulejsijni.so",
+            "**/libreact_nativemodule_core.so",
+            "**/libjscexecutor.so"
+    ]
+  }


this still need those days?, I created a rn turbo modules recently then migrated to nitro later, and i didn't got any symbol conflict as we used to have in the past

I've got these from @mrousavy's nitro template, @mrousavy is it safe to remove these? Also maybe we can make this to default for everyone?

I don't know, lol.

FYI I tried to build nitro without these, but I got an error mentioning some of these libs were duplicated so these are needed for nitro modules right now.

satya164 · 2024-12-06T19:26:06Z

.github/workflows/build-templates.yml

+      - name: Run nitro codegen
+        if: matrix.type == 'nitro-module'
+        run: |
+          yarn nitrogen


We need to run this manually?

Also contributing guide needs to include the workflow in case there are any new steps - like running this (and when is it needed)

packages/create-react-native-library/src/exampleApp/generateExampleApp.ts

packages/create-react-native-library/src/index.ts

packages/create-react-native-library/templates/common/CONTRIBUTING.md

packages/create-react-native-library/src/inform.ts

packages/create-react-native-library/templates/nitro-module/android/$CMakeLists.txt

packages/create-react-native-library/templates/nitro-module/$nitro.json

satya164 · 2024-12-06T19:51:47Z

packages/create-react-native-library/templates/native-common/android/build.gradle

@@ -49,7 +52,11 @@ def supportsNamespace() {

 android {
  if (supportsNamespace()) {
+<% if (project.nitro) { -%>
+    namespace "com.margelo.nitro.<%- project.package -%>"


isn't this the library's namespace? then it shouldn't have margelo.com since that's not the library's organization.

nitro codegen expects the kotlin files under the margelo namespace. We have to use margelo namespace until this is changed in nitrogen.
cc: @mrousavy

hmm this doesn't seem good. i wouldn't feel comfortable having my own library under margelo's domain.

Yea Nitro expects margelo.com namespaces for all Hybrid Objects.
This was to simplify the ProGuard annotations (if something is missing from com.margelo it was stripped out, which we explicitly deal with) - and also it makes cross-referencing Hybrid Objects from other Nitro Modules easier since we don't have to worry about different namespaces here.
Also, it's at least a little bit of branding 😄

i wouldn't feel comfortable having my own library under margelo's domain.

All the generated code from Turbo Modules are under facebook's domain. This is the same here, the generated code is under margelo domain.

Nitro expects registered hybrid objects to be under the same domain as well, not any custom configurable domain - which is the same as how it works in C++ Turbo Modules.
This was just to simplify the whole thing.

All the generated code from Turbo Modules are under facebook's domain. This is the same here, the generated code is under margelo domain.

Having the generated code or a specific part of the code vs the entire package like it's done here are quite different things. I'm sure bigger companies would have restrictions on using their namespace and publishing to their internal registries.

yea I understand, but this is not gonna change anytime soon as it might require larger changes in the Nitrogen codebase.

If a bigger company reaches out to me because they want to use Nitro but can't because of namespace restrictions, then I'll work on that with them. For now, I don't see a problem in it tbh.

packages/create-react-native-library/src/index.ts

satya164 · 2024-12-06T19:58:10Z

packages/create-react-native-library/templates/common/$package.json

@@ -72,7 +78,7 @@
  "devDependencies": {
    "@commitlint/config-conventional": "^17.0.2",
    "@evilmartians/lefthook": "^1.5.0",
-<% if (example === 'vanilla') { -%>
+<% if (example === 'vanilla' && !project.nitro) { -%>


It'll be more future-proof to add configs based on what type of project it is rather than what type of project it is not. That way adding a new type won't require adding new condition to each call site.

I think we should just add a key for the official module type and use it for these checks (here and other places) instead of !project.nitro.

I've introduced project.moduleConfig (which replaces project.module) as an union type of native-modules, turbo-modules, nitro-modules. This is the native modules api that we use.

packages/create-react-native-library/templates/common/README.md

…TING.md Co-authored-by: Marc Rousavy <[email protected]>

Co-authored-by: Marc Rousavy <[email protected]>

mrousavy reviewed Dec 6, 2024

View reviewed changes

atlj force-pushed the @atlj/nitro branch from 6c0eb0a to fbf8735 Compare December 6, 2024 17:24

rodgomesc reviewed Dec 6, 2024

View reviewed changes

satya164 requested changes Dec 6, 2024

View reviewed changes

satya164 reviewed Dec 6, 2024

View reviewed changes

packages/create-react-native-library/templates/common/README.md Show resolved Hide resolved

atlj force-pushed the @atlj/nitro branch 6 times, most recently from 5df5303 to 54d0fa5 Compare December 17, 2024 14:43

atlj added 18 commits December 19, 2024 15:24

feat: add script target

fc54b68

feat: add clean to the script target

79dd761

docs: script target

9f3c4e2

feat: rename script target to custom

5c92491

feat: remove cwd

2b6147a

feat: better logs

4fee364

feat: add nitro option to cli

0c0af16

fix: don't print using undefined react native version

693bb99

feat: add new arch to nitro modules

a8ca8d4

feat: apply the nitro template

f14e6e1

feat: create index and nitro files

b453fb1

feat: add required stuff to podspec

901c267

feat: add cmakelist

4b00668

feat: add cpp adapter

22cd1f3

feat: don't add codegen with nitro

26f8ca5

feat: don't add rnccli for nitro

b35b1d9

feat: add kotlin files

2b55c2d

feat: add swift code

f517a55

atlj and others added 20 commits December 19, 2024 15:44

fix: better spacing

40fc3cf

fix: correct the nitro paths

6d65dc4

chore: add nitro to CI

665095d

Update packages/create-react-native-library/templates/common/CONTRIBU…

fa9129f

…TING.md Co-authored-by: Marc Rousavy <[email protected]>

Update packages/create-react-native-library/src/inform.ts

3b5adea

Co-authored-by: Marc Rousavy <[email protected]>

docs: update comment

8fcd1d5

feat: add newline with nitro

8e1b185

feat: don't mention the nitro docs when user creates a lib

49a6d40

chore: remove unnecessary $ from file names

a27150d

feat: explain why nitro modules are needed as a dependency

1e2b705

feat: change nitro description

58d421e

refactor: remove package_array

8fffdf5

feat: remove unnecessary comment

960af29

refactor: introduce viewConfig and moduleConfig

946638c

add newline in nitro ts

ab5bfd1

fix: use the correct cpp path

5571d69

feat: only ignore jsi libs on nitro modules

4c59742

feat: ignore nitrogen

2822d28

feat: add nitrogen custom target

e37a334

feat: generate codegen code on lib ci

7e9ea53

atlj force-pushed the @atlj/nitro branch from 34cf2b2 to 7e9ea53 Compare December 19, 2024 12:50

atlj marked this pull request as ready for review December 19, 2024 12:50

atlj added 2 commits December 19, 2024 18:46

fix: remove nitrogen step

04a0b57

feat: only fetch the nitro version when the module type is nitro

53bd9fe

atlj changed the title ~~feat(crnl): support nitro modules~~ [WIP] feat(crnl): support nitro modules Dec 20, 2024

atlj added 2 commits December 20, 2024 11:39

feat: make nitro the second option and add an experimental label

9f995f2

feat: remove one doc link from contributing

b85d59d

atlj requested a review from satya164 December 20, 2024 08:59

atlj added 2 commits December 22, 2024 12:46

feat: pin the nitro modules peer dep

59d26e2

feat: update ios template with the latest version

4ab8e59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] feat(crnl): support nitro modules #721

[WIP] feat(crnl): support nitro modules #721

atlj commented Dec 6, 2024 •

edited

Loading

mrousavy commented Dec 6, 2024

mrousavy left a comment

mrousavy Dec 6, 2024

mrousavy Dec 6, 2024

rodgomesc Dec 6, 2024

atlj Dec 6, 2024

mrousavy Dec 7, 2024

atlj Dec 20, 2024

satya164 Dec 6, 2024

satya164 Dec 6, 2024

atlj Dec 17, 2024

satya164 Dec 18, 2024

mrousavy Dec 18, 2024

mrousavy Dec 18, 2024

satya164 Dec 18, 2024

mrousavy Dec 18, 2024

mrousavy Dec 18, 2024

satya164 Dec 6, 2024

atlj Dec 16, 2024

[WIP] feat(crnl): support nitro modules #721

Are you sure you want to change the base?

[WIP] feat(crnl): support nitro modules #721

Conversation

atlj commented Dec 6, 2024 • edited Loading

Summary

What's next?

Test plan

Example app

Published lib

mrousavy commented Dec 6, 2024

mrousavy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atlj commented Dec 6, 2024 •

edited

Loading