-
Notifications
You must be signed in to change notification settings - Fork 7
Derive tag on fork from .hermesversion in "react-native" #158
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "react-native-node-api": minor | ||
| --- | ||
|
|
||
| Derive the tag used to clone the React Native fork bringing Node-API support from the .hermesversion file in the react-native package. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,6 @@ import { prettyPath } from "../path-utils"; | |
| const HOST_PACKAGE_ROOT = path.resolve(__dirname, "../../.."); | ||
| // FIXME: make this configurable with reasonable fallback before public release | ||
| const HERMES_GIT_URL = "https://github.com/kraenhansen/hermes.git"; | ||
| const HERMES_GIT_TAG = "node-api-for-react-native-0.79.0"; | ||
|
|
||
| export const command = new Command("vendor-hermes") | ||
| .argument("[from]", "Path to a file inside the app package", process.cwd()) | ||
|
|
@@ -26,6 +25,27 @@ export const command = new Command("vendor-hermes") | |
| try { | ||
| const appPackageRoot = packageDirectorySync({ cwd: from }); | ||
| assert(appPackageRoot, "Failed to find package root"); | ||
| const reactNativePath = path.dirname( | ||
| require.resolve("react-native/package.json", { | ||
| // Ensures we'll be patching the React Native package actually used by the app | ||
| paths: [appPackageRoot], | ||
| }) | ||
| ); | ||
| const hermesVersionPath = path.join( | ||
| reactNativePath, | ||
| "sdks", | ||
| ".hermesversion" | ||
| ); | ||
| const hermesVersion = fs.readFileSync(hermesVersionPath, "utf8").trim(); | ||
| if (!silent) { | ||
| console.log(`Using Hermes version: ${hermesVersion}`); | ||
| } | ||
|
|
||
| const reactNativeJsiPath = path.join( | ||
| reactNativePath, | ||
| "ReactCommon/jsi/jsi/" | ||
| ); | ||
|
|
||
| const hermesPath = path.join(HOST_PACKAGE_ROOT, "hermes"); | ||
| if (force && fs.existsSync(hermesPath)) { | ||
| await oraPromise( | ||
|
|
@@ -40,42 +60,48 @@ export const command = new Command("vendor-hermes") | |
| ); | ||
| } | ||
| if (!fs.existsSync(hermesPath)) { | ||
| await oraPromise( | ||
| spawn( | ||
| "git", | ||
| [ | ||
| "clone", | ||
| "--recursive", | ||
| "--depth", | ||
| "1", | ||
| "--branch", | ||
| HERMES_GIT_TAG, | ||
| HERMES_GIT_URL, | ||
| hermesPath, | ||
| ], | ||
| const patchedTag = `node-api-${hermesVersion}`; | ||
| try { | ||
| await oraPromise( | ||
| spawn( | ||
| "git", | ||
| [ | ||
| "clone", | ||
| "--recursive", | ||
| "--depth", | ||
| "1", | ||
| "--branch", | ||
| patchedTag, | ||
| HERMES_GIT_URL, | ||
| hermesPath, | ||
| ], | ||
| { | ||
| outputMode: "buffered", | ||
| } | ||
| ), | ||
| { | ||
| outputMode: "buffered", | ||
| text: `Cloning custom Hermes into ${prettyPath(hermesPath)}`, | ||
| successText: "Cloned custom Hermes", | ||
| failText: (err) => | ||
| `Failed to clone custom Hermes: ${err.message}`, | ||
| isEnabled: !silent, | ||
| } | ||
| ), | ||
| { | ||
| text: `Cloning custom Hermes into ${prettyPath(hermesPath)}`, | ||
| successText: "Cloned custom Hermes", | ||
| failText: (err) => `Failed to clone custom Hermes: ${err.message}`, | ||
| isEnabled: !silent, | ||
| ); | ||
| } catch (error) { | ||
| if (error instanceof SpawnFailure) { | ||
| error.flushOutput("both"); | ||
| console.error( | ||
| `\n🛑 React Native uses the ${hermesVersion} tag and cloning our fork failed.`, | ||
| `Please see the Node-API package's peer dependency on "react-native" for supported versions.` | ||
| ); | ||
|
Comment on lines
+93
to
+96
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally, we would match on the output of the command, but that isn't yet supported by |
||
| process.exitCode = 1; | ||
| return; | ||
| } else { | ||
| throw error; | ||
| } | ||
| ); | ||
| } | ||
| } | ||
| const hermesJsiPath = path.join(hermesPath, "API/jsi/jsi"); | ||
| const reactNativePath = path.dirname( | ||
| require.resolve("react-native/package.json", { | ||
| // Ensures we'll be patching the React Native package actually used by the app | ||
| paths: [appPackageRoot], | ||
| }) | ||
| ); | ||
| const reactNativeJsiPath = path.join( | ||
| reactNativePath, | ||
| "ReactCommon/jsi/jsi/" | ||
| ); | ||
|
|
||
| assert( | ||
| fs.existsSync(hermesJsiPath), | ||
|
|
@@ -96,6 +122,7 @@ export const command = new Command("vendor-hermes") | |
| ); | ||
| console.log(hermesPath); | ||
| } catch (error) { | ||
| process.exitCode = 1; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turns out this didn't result in a failure. |
||
| if (error instanceof SpawnFailure) { | ||
| error.flushOutput("both"); | ||
| } | ||
|
|
||
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.
Wrap the
.hermesversionread in a try/catch to surface a clear error if the file is missing or unreadable, e.g., ‘Failed to read Hermes version from .hermesversion: ’.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 is very unlikely to happen. If I were to do this, I would call a function instead of using a mutable var.