Skip to content

Conversation

tconns
Copy link

@tconns tconns commented Aug 5, 2025

Summary

Because RN >= 0.80 has extracted the example out of the core

Changelog

Test Plan

Because RN >= 0.80 has extracted the example out of the core
Copy link
Contributor

@dougg0k dougg0k left a comment

Choose a reason for hiding this comment

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

You should do a check before, if the previous path are not available, then you use the new path. Otherwise it will be compatible only with the newer version of RN.

@tconns
Copy link
Author

tconns commented Aug 5, 2025

You should do a check before, if the previous path are not available, then you use the new path. Otherwise it will be compatible only with the newer version of RN.

But the check just above was already working with RN versions below 0.80, wasn’t it?

@ildfreelancer
Copy link

@tconns your fix seems wrong

it should be

reactNativeDir = Paths.get(nodeModulesDir.toString(), "react-native/android")
if (!reactNativeDir.toFile().exists()) {
  reactNativeDir = Paths.get(nodeModulesDir.toString(), "react-native/ReactAndroid")
}

@numandev1
Copy link
Owner

i also agree with @ildfreelancer

@numandev1
Copy link
Owner

@dougg0k I sent you an invite on GitHub, can you check?

@dougg0k
Copy link
Contributor

dougg0k commented Aug 6, 2025

Thanks @numandev1 but I am good as collab / contrib only, when I can.

@dougg0k
Copy link
Contributor

dougg0k commented Aug 9, 2025

The current impl are weird, even though the first are being used for the project, a different path is used for the example project.

Then it's checked again for the actual node_modules. It seem to contain duplication in there.

https://github.com/numandev1/react-native-keys/blob/main/android/build.gradle#L85


Perhaps it should be.

// Find node_modules inside the example project
def nodeModulesDir = Paths.get(basePath.toString(), "node_modules")
def reactNativeDir = Paths.get(nodeModulesDir.toString(), "react-native/android")
if (!reactNativeDir.toFile().exists()) {
  reactNativeDir = Paths.get(nodeModulesDir.toString(), "react-native/ReactAndroid")
}
if (nodeModulesDir.toFile().exists() && reactNativeDir.toFile().exists()) {
  found = true
}

Removing


if(!found){
  // Node's module resolution algorithm searches up to the root directory,
  // after which the base path will be null
  while (basePath) {
    nodeModulesDir = Paths.get(basePath.toString(), "node_modules")
    reactNativeDir = Paths.get(nodeModulesDir.toString(), "react-native/android")
    if (nodeModulesDir.toFile().exists() && reactNativeDir.toFile().exists()) {
      found = true
      break;
    }
    basePath = basePath.getParent()
  }
}

Since the example project should have no ties to the project. But it's own android folder.

@JerakRus
Copy link

JerakRus commented Aug 28, 2025

Just as an interesting note: when I approved this PR #102, I made a mistake and applied my own patch to the exact same lines that are changed in this current PR. Now that I've updated rn-keys to 0.7.12 and removed my patch, it's broken again. So it turns out that this diff is actually correct and functional:


if(!found){
  // Node's module resolution algorithm searches up to the root directory,
  // after which the base path will be null
  while (basePath) {
    nodeModulesDir = Paths.get(basePath.toString(), "node_modules")
    reactNativeDir = Paths.get(nodeModulesDir.toString(), "react-native/android")
    if (nodeModulesDir.toFile().exists() && reactNativeDir.toFile().exists()) {
      found = true
      break;
    }
    basePath = basePath.getParent()
  }
}

And the previous PR (#102) should be rolled back.

@dougg0k
Copy link
Contributor

dougg0k commented Aug 28, 2025

functional are not necessarily correct, but just functional.

that is functional because before that, it is considering the supposed example project, but it should have no need.

@dougg0k
Copy link
Contributor

dougg0k commented Aug 30, 2025

What is probably missing here due to the example project is another check of the same, done in my PR for newer RN versions.

#118

https://github.com/numandev1/react-native-keys/blob/main/android/build.gradle#L96
https://github.com/numandev1/react-native-keys/blob/main/android/build.gradle#L84

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.

5 participants