refactor hash by js side #514
Conversation
WalkthroughTwo new Node.js scripts were added: Changes
Sequence Diagram(s)sequenceDiagram
participant Metro as Metro Bundler
participant Plugin as Bundle Metadata Plugin
participant FS as File System
participant HermesWrapper as hermesc-wrapper
participant Hermes as Real hermesc
Metro->>Plugin: metadataSerializer(entryPoint, preModules, graph, options)
rect rgb(220,230,240)
Note over Plugin,FS: Setup hermesc wrapper locations
Plugin->>FS: create/write wrapper scripts
end
rect rgb(240,230,220)
Note over Plugin: Build & Hash
Plugin->>Plugin: generate base JS bundle
Plugin->>Plugin: calculate SHA-256 (contentHash)
Plugin->>FS: write bundle-hash.json (contentHash + timestamp)
Plugin->>Plugin: inject metadata snippet + trailing comment
Plugin-->>Metro: return augmented bundle
end
Metro->>HermesWrapper: invoke hermesc (wrapped) with args
HermesWrapper->>Hermes: spawn real hermesc (forward args)
rect rgb(230,240,230)
Note over HermesWrapper,FS: Post-process on success
Hermes->>HermesWrapper: exit 0 (compile success)
HermesWrapper->>FS: read bundle-hash.json -> contentHash
HermesWrapper->>FS: read HBC output file
HermesWrapper->>HermesWrapper: append RNUPDATE + JSON + length + RNUPDATE
HermesWrapper->>FS: write modified HBC
HermesWrapper-->>Metro: exit 0
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
scripts/hermesc-wrapper.js (1)
31-40: Avoid arbitrary 500ms sleep; invoke post-processing immediately after hermesc closesThe delay is brittle and can prolong builds. Call the processor directly on successful close.
- if (code === 0 && isCompileOperation && outputFile) { - console.log(`[Hermesc Wrapper] 🔄 Post-processing HBC file: ${outputFile}`); - - setTimeout(() => { - processHBCFile(outputFile); - }, 500); - } else { + if (code === 0 && isCompileOperation && outputFile) { + console.log(`[Hermesc Wrapper] 🔄 Post-processing HBC file: ${outputFile}`); + processHBCFile(outputFile); + } else { process.exit(code); }scripts/process-hbc.js (2)
65-78: Broaden Android discovery (add debug paths)Include common debug task outputs to improve usability.
const possiblePaths = [ 'android/app/build/generated/assets/react/release/index.android.bundle', 'android/app/build/generated/assets/createBundleReleaseJsAndAssets/index.android.bundle', 'android/app/src/main/assets/index.android.bundle', + 'android/app/build/generated/assets/react/debug/index.android.bundle', + 'android/app/build/generated/assets/createBundleDebugJsAndAssets/index.android.bundle', ];
82-96: Broaden iOS discovery (simulator/debug variants and generic .app scan)Add simulator paths; your fallback scan is good—keep it.
const possiblePaths = [ `ios/${projectName}.app/main.jsbundle`, `ios/build/Build/Products/Release-iphoneos/${projectName}.app/main.jsbundle`, `ios/build/Build/Products/Debug-iphoneos/${projectName}.app/main.jsbundle`, + `ios/build/Build/Products/Release-iphonesimulator/${projectName}.app/main.jsbundle`, + `ios/build/Build/Products/Debug-iphonesimulator/${projectName}.app/main.jsbundle`, 'ios/main.jsbundle', 'ios/build/Build/Products/Release-iphoneos/main.jsbundle', + 'ios/build/Build/Products/Release-iphonesimulator/main.jsbundle', + 'ios/build/Build/Products/Debug-iphonesimulator/main.jsbundle', ];Also applies to: 97-108
scripts/bundle-metadata-plugin.js (2)
129-139: Broaden hermesc locations (Linux support) and summarize correctlyAdd Linux path so Android builds on Linux are instrumented too.
const hermescLocations = [ { path: path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/hermesc'), name: 'Node Modules' }, + { + path: path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/linux64-bin/hermesc'), + name: 'Node Modules (Linux)' + }, { path: path.join(PROJECT_ROOT, 'ios/Pods/hermes-engine/destroot/bin/hermesc'), name: 'iOS Pods' } ];Also applies to: 141-153
13-19: Safer global metadata injectionAvoid leaking a top-level var; assign to globalThis to reduce collision risk.
function generateMetadataInjection(contentHash) { - return `// Auto-injected bundle metadata by Metro plugin - var __BUNDLE_METADATA__ = { - contentHash: '${contentHash}' - }; -`; + return `// Auto-injected bundle metadata by Metro plugin + (function(){ try { (typeof globalThis!=='undefined'?globalThis:global).__BUNDLE_METADATA__ = { contentHash: '${contentHash}' }; } catch(e){} })(); +`; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scripts/bundle-metadata-plugin.js(1 hunks)scripts/hermesc-wrapper.js(1 hunks)scripts/process-hbc.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
scripts/process-hbc.js (2)
scripts/bundle-metadata-plugin.js (8)
fs(2-2)path(3-3)PROJECT_ROOT(4-4)contentHash(161-161)hashFilePath(164-164)hashData(167-170)successCount(141-141)success(145-145)scripts/hermesc-wrapper.js (14)
fs(5-5)require(3-3)path(4-4)PROJECT_ROOT(6-6)hbcBuffer(69-69)metadata(71-71)metadataJson(72-72)MAGIC(74-74)jsonBuffer(75-75)lengthBuffer(76-76)finalBuffer(79-85)hashFilePath(46-46)hashData(63-63)hashData(64-64)
scripts/bundle-metadata-plugin.js (2)
scripts/hermesc-wrapper.js (7)
require(3-3)fs(5-5)path(4-4)PROJECT_ROOT(6-6)hashFilePath(46-46)hashData(63-63)hashData(64-64)scripts/process-hbc.js (8)
fs(3-3)path(4-4)PROJECT_ROOT(5-5)successCount(142-142)success(144-144)hashFilePath(118-118)hashData(125-125)hashData(126-126)
scripts/hermesc-wrapper.js (2)
scripts/bundle-metadata-plugin.js (6)
path(3-3)fs(2-2)PROJECT_ROOT(4-4)hashFilePath(164-164)hashData(167-170)contentHash(161-161)scripts/process-hbc.js (13)
path(4-4)fs(3-3)PROJECT_ROOT(5-5)hashFilePath(118-118)hashData(125-125)hashData(126-126)hbcBuffer(13-13)metadata(15-17)metadataJson(18-18)MAGIC(20-20)jsonBuffer(21-21)lengthBuffer(22-22)finalBuffer(25-31)
| const shellScript = `#!/bin/bash | ||
| # Hermesc wrapper script - auto-generated | ||
| # This script calls the Node.js wrapper which handles post-processing | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | ||
| WRAPPER_SCRIPT="${wrapperSourcePath}" | ||
|
|
||
| find_node() { | ||
| if command -v node >/dev/null 2>&1; then | ||
| command -v node | ||
| return 0 | ||
| fi | ||
|
|
||
| local NODE_PATHS=( | ||
| "/usr/local/bin/node" | ||
| "/opt/homebrew/bin/node" | ||
| "$HOME/.nvm/versions/node/$(ls -t "$HOME/.nvm/versions/node" 2>/dev/null | head -1)/bin/node" | ||
| "/usr/bin/node" | ||
| ) | ||
|
|
||
| for node_path in "\${NODE_PATHS[@]}"; do | ||
| if [ -x "$node_path" ]; then | ||
| echo "$node_path" | ||
| return 0 | ||
| fi | ||
| done | ||
|
|
||
| echo "Error: node executable not found" >&2 | ||
| echo "Please ensure Node.js is installed and accessible" >&2 | ||
| exit 1 | ||
| } | ||
|
|
||
| NODE_BIN=$(find_node) | ||
| exec "$NODE_BIN" "$WRAPPER_SCRIPT" "$@" | ||
| `; |
There was a problem hiding this comment.
Shell wrapper must pass the hermesc directory to the Node wrapper
Export HERMESC_DIR so hermesc-wrapper can spawn the correct local _hermesc (Pods vs node_modules, Linux).
- const shellScript = `#!/bin/bash
+ const shellScript = `#!/bin/bash
# Hermesc wrapper script - auto-generated
# This script calls the Node.js wrapper which handles post-processing
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
WRAPPER_SCRIPT="${wrapperSourcePath}"
@@
NODE_BIN=$(find_node)
-exec "$NODE_BIN" "$WRAPPER_SCRIPT" "$@"
+export HERMESC_DIR="$SCRIPT_DIR"
+exec "$NODE_BIN" "$WRAPPER_SCRIPT" "$@"
`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const shellScript = `#!/bin/bash | |
| # Hermesc wrapper script - auto-generated | |
| # This script calls the Node.js wrapper which handles post-processing | |
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | |
| WRAPPER_SCRIPT="${wrapperSourcePath}" | |
| find_node() { | |
| if command -v node >/dev/null 2>&1; then | |
| command -v node | |
| return 0 | |
| fi | |
| local NODE_PATHS=( | |
| "/usr/local/bin/node" | |
| "/opt/homebrew/bin/node" | |
| "$HOME/.nvm/versions/node/$(ls -t "$HOME/.nvm/versions/node" 2>/dev/null | head -1)/bin/node" | |
| "/usr/bin/node" | |
| ) | |
| for node_path in "\${NODE_PATHS[@]}"; do | |
| if [ -x "$node_path" ]; then | |
| echo "$node_path" | |
| return 0 | |
| fi | |
| done | |
| echo "Error: node executable not found" >&2 | |
| echo "Please ensure Node.js is installed and accessible" >&2 | |
| exit 1 | |
| } | |
| NODE_BIN=$(find_node) | |
| exec "$NODE_BIN" "$WRAPPER_SCRIPT" "$@" | |
| `; | |
| const shellScript = `#!/bin/bash | |
| # Hermesc wrapper script - auto-generated | |
| # This script calls the Node.js wrapper which handles post-processing | |
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | |
| WRAPPER_SCRIPT="${wrapperSourcePath}" | |
| find_node() { | |
| if command -v node >/dev/null 2>&1; then | |
| command -v node | |
| return 0 | |
| fi | |
| local NODE_PATHS=( | |
| "/usr/local/bin/node" | |
| "/opt/homebrew/bin/node" | |
| "$HOME/.nvm/versions/node/$(ls -t "$HOME/.nvm/versions/node" 2>/dev/null | head -1)/bin/node" | |
| "/usr/bin/node" | |
| ) | |
| for node_path in "\${NODE_PATHS[@]}"; do | |
| if [ -x "$node_path" ]; then | |
| echo "$node_path" | |
| return 0 | |
| fi | |
| done | |
| echo "Error: node executable not found" >&2 | |
| echo "Please ensure Node.js is installed and accessible" >&2 | |
| exit 1 | |
| } | |
| NODE_BIN=$(find_node) | |
| export HERMESC_DIR="$SCRIPT_DIR" | |
| exec "$NODE_BIN" "$WRAPPER_SCRIPT" "$@" | |
| `; |
🤖 Prompt for AI Agents
In scripts/bundle-metadata-plugin.js around lines 53 to 87, the generated shell
wrapper doesn't export HERMESC_DIR so the hermesc Node wrapper can't locate the
correct local _hermesc (Pods vs node_modules, Linux); update the script to
export HERMESC_DIR pointing to the hermesc directory (for example export
HERMESC_DIR="$SCRIPT_DIR" or another computed path if hermesc lives elsewhere
relative to SCRIPT_DIR) before invoking the Node wrapper so hermesc-wrapper can
spawn the correct _hermesc.
| function metadataSerializer(entryPoint, preModules, graph, options) { | ||
| setupHermescWrapper(); | ||
| const baseJSBundle = require('metro/src/DeltaBundler/Serializers/baseJSBundle'); | ||
| const bundleToString = require('metro/src/lib/bundleToString'); | ||
| const bundle = baseJSBundle(entryPoint, preModules, graph, options); | ||
| const { code: bundleCode } = bundleToString(bundle); | ||
| const contentHash = calculateContentHash(bundleCode); | ||
| const metadataInjection = generateMetadataInjection(contentHash); | ||
| const metadataComment = generateMetadataComment(contentHash); | ||
| const hashFilePath = path.join(PROJECT_ROOT, 'bundle-hash.json'); | ||
|
|
||
| try { | ||
| const hashData = { | ||
| contentHash, | ||
| timestamp: new Date().toISOString(), | ||
| }; | ||
| fs.writeFileSync(hashFilePath, JSON.stringify(hashData, null, 2)); | ||
| console.log(`✅ [Metro] Saved hash to: ${hashFilePath}`); | ||
| console.log(`🔐 [Metro] Hash: ${contentHash.slice(0, 16)}...`); | ||
| } catch (error) { | ||
| console.error('❌ [Metro] Failed to save hash file:', error); | ||
| } | ||
|
|
||
| return bundleCode + metadataInjection + metadataComment; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Metro internals can change; verify API compatibility for your RN/Metro versions
Reaching into metro/src is version-sensitive. Please confirm the serializer API for your supported RN range.
🏁 Script executed:
#!/bin/bash
# Inspect installed Metro version and check serializer paths
jq -r '.dependencies.metro // .devDependencies.metro' package.json 2>/dev/null || true
fd -t f 'baseJSBundle.js' node_modules/metro -a
fd -t f 'bundleToString.js' node_modules/metro -a
# Optional: grep exports for function signatures
rg -nP 'function\s+baseJSBundle|exports\.baseJSBundle' node_modules/metro -n -C2Length of output: 275
🏁 Script executed:
#!/bin/bash
# Read package.json to get Metro version
echo "=== Metro version in package.json ==="
cat package.json | jq -r '.dependencies.metro // .devDependencies.metro // "NOT FOUND"'
echo -e "\n=== React Native version ==="
cat package.json | jq -r '.dependencies["react-native"] // .devDependencies["react-native"] // "NOT FOUND"'
echo -e "\n=== Search codebase for Metro version constraints or compatibility notes ==="
rg -i "metro.*version|compatibility|baseJSBundle|bundleToString" --type=js --type=json -C2
echo -e "\n=== Check for package-lock or yarn.lock for pinned versions ==="
head -50 package-lock.json 2>/dev/null | grep -A5 '"metro"' || echo "No package-lock found or metro section"
echo -e "\n=== Search for other Metro internal imports in codebase ==="
rg "from 'metro/src|require.*metro/src" --type=js -C1Length of output: 1472
🌐 Web query:
React Native 0.73 Metro version bundler serializer API
💡 Result:
Short answer — in React Native 0.73 Metro’s serializer is the standard Metro serializer but configured via metro.config.js using serializer.customSerializer with the async signature:
config.serializer.customSerializer = async (entryPoint, preModules, graph, options) => { ... }
This is the same hook used by the rnx-kit/MetroSerializer plugin system (MetroSerializer(...) returns a customSerializer you can assign) and Metro’s docs describe serialization as the final bundling stage and expose the bundling APIs. See:
- RN 0.73 release notes (Metro changes / config format required). [1]
- rnx-kit metro-serializer + plugin examples showing the customSerializer(entryPoint, preModules, graph, options) usage. [2]
- Metro docs (concepts/serialization and Metro API for bundling). [3]
References
[1] React Native 0.73 release notes — reactnative.dev.
[2] @rnx-kit/metro-serializer docs (usage & customSerializer examples).
[3] Metro documentation — Concepts and Bundling API.
🌐 Web query:
metro baseJSBundle bundleToString public API stability
💡 Result:
Short answer: bundleToString (and the BaseJSBundle/ baseJSBundle serializer internals) are internal implementation details of Metro, not a supported public API — they can and have changed between Metro versions, so you should not rely on them for stable public use. Use Metro's documented serializer/config hooks instead (or pin Metro to a specific version if you must depend on internals). [1][2][3]
Sources:
- Metro repo / implementation (serializer code lives in Metro's source — internal serializer functions are not documented public APIs). [1]
- Metro documentation (public config/serializer hooks are documented; internals like baseJSBundle/bundleToString are implementation details). [2]
- Third‑party writeups showing baseJSBundle / bundleToString are internal parts of Metro's serializer (illustrates how they’re used internally). [3]
Refactor to use Metro's documented serializer API instead of internal implementation details
The imports from metro/src/DeltaBundler/Serializers/baseJSBundle and metro/src/lib/bundleToString are internal Metro implementation details, not public APIs. These can change between Metro versions without notice. React Native 0.73 provides a documented public API for this: configure a custom serializer via metro.config.js using the serializer.customSerializer hook with the signature async (entryPoint, preModules, graph, options) => { ... }. Refactor this code to use that public hook instead of reaching into Metro's internals.
🤖 Prompt for AI Agents
scripts/bundle-metadata-plugin.js around lines 155 to 179: this function
currently requires Metro internals (baseJSBundle, bundleToString) — refactor it
to be used via Metro's documented serializer.customSerializer hook instead of
importing internal modules. Change the function to accept/delegate to Metro's
default serializer (i.e., make it either an async factory that takes
defaultSerializer or export a function that receives defaultSerializer when
called from metro.config.js), call that default serializer to produce the bundle
(await defaultSerializer(entryPoint, preModules, graph, options)), derive
bundleCode/from the returned bundle, compute and write the contentHash and
inject metadata exactly as before, and update metro.config.js to pass Metro's
default serializer into this plugin via serializer.customSerializer so no
internal metro/* requires remain.
| const realHermescPath = path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/_hermesc'); | ||
| const args = process.argv.slice(2); |
There was a problem hiding this comment.
Blocker: wrapper hard-codes a single macOS hermesc path; breaks for iOS Pods/Linux and mismatches the wrapped location
Derive the real hermesc from the invoking hermesc dir (exported by the shell wrapper) and fall back to known locations. Without this, invocations from Pods or Linux fail or compile with the wrong binary.
Apply:
-const realHermescPath = path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/_hermesc');
+function resolveRealHermescPath() {
+ const candidates = [];
+ if (process.env.HERMESC_DIR) {
+ candidates.push(path.join(process.env.HERMESC_DIR, '_hermesc'));
+ }
+ candidates.push(
+ path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/_hermesc'),
+ path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/linux64-bin/_hermesc'),
+ path.join(PROJECT_ROOT, 'ios/Pods/hermes-engine/destroot/bin/_hermesc'),
+ );
+ return candidates.find(p => fs.existsSync(p));
+}
+const realHermescPath = resolveRealHermescPath();
+if (!realHermescPath) {
+ console.error('[Hermesc Wrapper] ❌ Could not locate _hermesc (set HERMESC_DIR or install Hermes).');
+ process.exit(1);
+}
+console.log(`[Hermesc Wrapper] Using hermesc: ${realHermescPath}`);Committable suggestion skipped: line range outside the PR's diff.
| try { | ||
| const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8')); | ||
| const { contentHash } = hashData; | ||
|
|
||
| console.log(`[Hermesc Wrapper] 📝 Injecting metadata into HBC...`); | ||
| console.log(`[Hermesc Wrapper] Hash: ${contentHash.slice(0, 16)}...`); | ||
|
|
||
| const hbcBuffer = fs.readFileSync(hbcFilePath); | ||
|
|
||
| const metadata = { contentHash }; | ||
| const metadataJson = JSON.stringify(metadata); | ||
|
|
||
| const MAGIC = Buffer.from('RNUPDATE', 'utf8'); | ||
| const jsonBuffer = Buffer.from(metadataJson, 'utf8'); | ||
| const lengthBuffer = Buffer.alloc(4); | ||
| lengthBuffer.writeUInt32LE(jsonBuffer.length); | ||
|
|
||
| const finalBuffer = Buffer.concat([ | ||
| hbcBuffer, | ||
| MAGIC, | ||
| jsonBuffer, | ||
| lengthBuffer, | ||
| MAGIC, | ||
| ]); | ||
|
|
||
| fs.writeFileSync(hbcFilePath, finalBuffer); |
There was a problem hiding this comment.
Make metadata appends idempotent and guard against missing/invalid hash
Repeated runs currently re-append RNUPDATE blocks and bloat the HBC. Also, slice() on an undefined hash can throw.
try {
- const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8'));
- const { contentHash } = hashData;
+ const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8'));
+ const { contentHash } = hashData || {};
+ if (typeof contentHash !== 'string' || !contentHash.length) {
+ console.warn('[Hermesc Wrapper] ⚠️ Invalid contentHash in bundle-hash.json; skipping.');
+ process.exit(0);
+ return;
+ }
@@
- const hbcBuffer = fs.readFileSync(hbcFilePath);
+ let hbcBuffer = fs.readFileSync(hbcFilePath);
@@
- const finalBuffer = Buffer.concat([
- hbcBuffer,
+ // Strip existing RNUPDATE footer if present (idempotent)
+ function stripRNUpdateFooter(buf, MAGIC) {
+ const m = MAGIC.length;
+ if (buf.length < (m * 2 + 4)) return { base: buf, stripped: false };
+ const endMagicStart = buf.length - m;
+ if (!buf.slice(endMagicStart).equals(MAGIC)) return { base: buf, stripped: false };
+ const lenStart = endMagicStart - 4;
+ if (lenStart < 0) return { base: buf, stripped: false };
+ const jsonLen = buf.readUInt32LE(lenStart);
+ const startMagicStart = lenStart - jsonLen - m;
+ if (startMagicStart < 0) return { base: buf, stripped: false };
+ if (!buf.slice(startMagicStart, startMagicStart + m).equals(MAGIC)) return { base: buf, stripped: false };
+ return { base: buf.slice(0, startMagicStart), stripped: true };
+ }
+ const { base: baseBuffer } = stripRNUpdateFooter(hbcBuffer, MAGIC);
+
+ const finalBuffer = Buffer.concat([
+ baseBuffer,
MAGIC,
jsonBuffer,
lengthBuffer,
MAGIC,
]);| console.log(`[Hermesc Wrapper] ✅ Successfully injected metadata into: ${hbcFilePath}`); | ||
| console.log(`[Hermesc Wrapper] 🧹 Cleaning up hash file...`); | ||
|
|
||
| process.exit(0); |
There was a problem hiding this comment.
Misleading log: no cleanup actually happens
Either remove the message or implement opt-in cleanup. Recommend removing to avoid confusion.
- console.log(`[Hermesc Wrapper] 🧹 Cleaning up hash file...`);🤖 Prompt for AI Agents
In scripts/hermesc-wrapper.js around lines 89 to 92, the log states "Cleaning up
hash file..." but no cleanup is performed; remove or replace this misleading
console.log and either delete the message entirely or implement an opt-in
cleanup (e.g., check a flag/env var and unlink the hash file before exiting).
Update the log to reflect the actual behavior if you choose not to implement
cleanup.
scripts/process-hbc.js
Outdated
| const hbcBuffer = await fs.promises.readFile(hbcPath); | ||
|
|
||
| const metadata = { | ||
| contentHash | ||
| }; | ||
| const metadataJson = JSON.stringify(metadata); | ||
|
|
||
| const MAGIC = Buffer.from('RNUPDATE', 'utf8'); | ||
| const jsonBuffer = Buffer.from(metadataJson, 'utf8'); | ||
| const lengthBuffer = Buffer.alloc(4); | ||
| lengthBuffer.writeUInt32LE(jsonBuffer.length); | ||
|
|
||
| const finalBuffer = Buffer.concat([ | ||
| hbcBuffer, | ||
| MAGIC, | ||
| jsonBuffer, | ||
| lengthBuffer, | ||
| MAGIC, | ||
| ]); | ||
|
|
||
| await fs.promises.writeFile(hbcPath, finalBuffer); | ||
| console.log(`[Process HBC] ✅ Appended metadata to: ${hbcPath}`); | ||
| console.log(`[Process HBC] Hash: ${contentHash.slice(0, 16)}...`); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Idempotent footer and stronger validation inside appendMetadataToHBC
Ensure we don’t endlessly append RNUPDATE blocks and fail early on bad inputs.
- const hbcBuffer = await fs.promises.readFile(hbcPath);
+ let hbcBuffer = await fs.promises.readFile(hbcPath);
@@
- const finalBuffer = Buffer.concat([
- hbcBuffer,
+ function stripRNUpdateFooter(buf, MAGIC) {
+ const m = MAGIC.length;
+ if (buf.length < (m * 2 + 4)) return { base: buf, stripped: false };
+ const endMagicStart = buf.length - m;
+ if (!buf.slice(endMagicStart).equals(MAGIC)) return { base: buf, stripped: false };
+ const lenStart = endMagicStart - 4;
+ if (lenStart < 0) return { base: buf, stripped: false };
+ const jsonLen = buf.readUInt32LE(lenStart);
+ const startMagicStart = lenStart - jsonLen - m;
+ if (startMagicStart < 0) return { base: buf, stripped: false };
+ if (!buf.slice(startMagicStart, startMagicStart + m).equals(MAGIC)) return { base: buf, stripped: false };
+ return { base: buf.slice(0, startMagicStart), stripped: true };
+ }
+ const { base: baseBuffer } = stripRNUpdateFooter(hbcBuffer, MAGIC);
+
+ const finalBuffer = Buffer.concat([
+ baseBuffer,
MAGIC,
jsonBuffer,
lengthBuffer,
MAGIC,
]);🤖 Prompt for AI Agents
In scripts/process-hbc.js around lines 13–37, the current append logic will
blindly append RNUPDATE blocks and lacks input validation; update it to (1)
validate inputs up front (ensure contentHash is a non-empty string of expected
length/hex format and hbcPath file exists and is at least big enough to contain
a footer), (2) inspect the existing file tail before appending: read the last 8
bytes and if they equal the MAGIC ('RNUPDATE') skip appending (idempotent) or,
for stronger validation, read the 4-byte little-endian length just before that
to verify a valid JSON block exists and only skip if that metadata is
well-formed, and (3) throw/return an error early for invalid inputs or corrupted
existing footer instead of appending. Ensure these checks occur before creating
finalBuffer and writing the file.
scripts/process-hbc.js
Outdated
| async function main() { | ||
| const platform = process.argv[2] || 'android'; | ||
| console.log(`[Process HBC] Platform: ${platform}`); | ||
| console.log(`[Process HBC] Project root: ${PROJECT_ROOT}`); | ||
| const hashFilePath = path.join(PROJECT_ROOT, 'bundle-hash.json'); | ||
|
|
||
| if (!fs.existsSync(hashFilePath)) { | ||
| console.error(`[Process HBC] ❌ Hash file not found: ${hashFilePath}`); | ||
| console.error('[Process HBC] Make sure Metro bundler has run with the custom serializer.'); | ||
| process.exit(1); | ||
| } | ||
| const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8')); | ||
| const { contentHash } = hashData; | ||
| console.log(`[Process HBC] Content hash: ${contentHash.slice(0, 16)}...`); | ||
| const hbcFiles = findHbcFiles(platform); |
There was a problem hiding this comment.
Validate hash manifest before use
Guard against missing/invalid contentHash to avoid runtime errors.
- const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8'));
- const { contentHash } = hashData;
- console.log(`[Process HBC] Content hash: ${contentHash.slice(0, 16)}...`);
+ const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8'));
+ const { contentHash } = hashData || {};
+ if (typeof contentHash !== 'string' || !contentHash.length) {
+ console.error('[Process HBC] ❌ Invalid contentHash in bundle-hash.json');
+ process.exit(1);
+ }
+ console.log(`[Process HBC] Content hash: ${contentHash.slice(0, 16)}...`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function main() { | |
| const platform = process.argv[2] || 'android'; | |
| console.log(`[Process HBC] Platform: ${platform}`); | |
| console.log(`[Process HBC] Project root: ${PROJECT_ROOT}`); | |
| const hashFilePath = path.join(PROJECT_ROOT, 'bundle-hash.json'); | |
| if (!fs.existsSync(hashFilePath)) { | |
| console.error(`[Process HBC] ❌ Hash file not found: ${hashFilePath}`); | |
| console.error('[Process HBC] Make sure Metro bundler has run with the custom serializer.'); | |
| process.exit(1); | |
| } | |
| const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8')); | |
| const { contentHash } = hashData; | |
| console.log(`[Process HBC] Content hash: ${contentHash.slice(0, 16)}...`); | |
| const hbcFiles = findHbcFiles(platform); | |
| async function main() { | |
| const platform = process.argv[2] || 'android'; | |
| console.log(`[Process HBC] Platform: ${platform}`); | |
| console.log(`[Process HBC] Project root: ${PROJECT_ROOT}`); | |
| const hashFilePath = path.join(PROJECT_ROOT, 'bundle-hash.json'); | |
| if (!fs.existsSync(hashFilePath)) { | |
| console.error(`[Process HBC] ❌ Hash file not found: ${hashFilePath}`); | |
| console.error('[Process HBC] Make sure Metro bundler has run with the custom serializer.'); | |
| process.exit(1); | |
| } | |
| const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8')); | |
| const { contentHash } = hashData || {}; | |
| if (typeof contentHash !== 'string' || !contentHash.length) { | |
| console.error('[Process HBC] ❌ Invalid contentHash in bundle-hash.json'); | |
| process.exit(1); | |
| } | |
| console.log(`[Process HBC] Content hash: ${contentHash.slice(0, 16)}...`); | |
| const hbcFiles = findHbcFiles(platform); |
🤖 Prompt for AI Agents
In scripts/process-hbc.js around lines 114 to 128, the code assumes
bundle-hash.json contains a valid contentHash and slices it directly which can
crash if the file is missing, malformed, or contentHash is absent/not a string;
add validation and error handling: wrap JSON.parse in try/catch to detect
malformed JSON, verify hashData and that typeof contentHash === 'string' and
contentHash.length > 0 (or a minimum length) before using .slice, log a clear
error (including the file path and parsed content if helpful) and call
process.exit(1) when validation fails so the script fails fast with a helpful
message.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
scripts/bundle-metadata-plugin.js (2)
53-87: Shell wrapper must export HERMESC_DIR so Node wrapper can find the real _hermesc
Without this, the Node wrapper can't resolve the correct binary for Pods vs node_modules vs Linux. Export the hermesc directory before exec.SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" WRAPPER_SCRIPT="${wrapperSourcePath}" +# Pass hermesc dir to the Node wrapper so it can resolve the colocated _hermesc +export HERMESC_DIR="$SCRIPT_DIR" + find_node() { @@ NODE_BIN=$(find_node) -exec "$NODE_BIN" "$WRAPPER_SCRIPT" "$@" +exec "$NODE_BIN" "$WRAPPER_SCRIPT" "$@"
157-163: Avoid Metro internals; refactor to a public customSerializer wrapper
Direct requires from metro/src are unstable across versions. Expose a factory that composes with the default serializer passed from metro.config.js (or via @rnx-kit/metro-serializer).Example change (plugin):
-function metadataSerializer(entryPoint, preModules, graph, options) { - setupHermescWrapper(); - const baseJSBundle = require('metro/src/DeltaBundler/Serializers/baseJSBundle'); - const bundleToString = require('metro/src/lib/bundleToString'); - const bundle = baseJSBundle(entryPoint, preModules, graph, options); - const { code: bundleCode } = bundleToString(bundle); +async function createMetadataSerializer(defaultSerializer) { + return async (entryPoint, preModules, graph, options) => { + setupHermescWrapper(); + // Call the provided default serializer (string or { code, ... }) + const out = await defaultSerializer(entryPoint, preModules, graph, options); + const bundleCode = typeof out === 'string' ? out : out?.code; + if (typeof bundleCode !== 'string') { + throw new Error('[Metro] Default serializer must return a code string or an object with .code'); + } + // ... keep logic below (hash, inject, write file) using bundleCode +``` And export: ```diff -module.exports = { - metadataSerializer, -}; +module.exports = { createMetadataSerializer };metro.config.js usage (no metro internals in the plugin):
// metro.config.js const defaultSerializer = require('metro/src/DeltaBundler/Serializers/baseJSBundle'); // or rnx-kit compose const { createMetadataSerializer } = require('react-native-update/scripts/bundle-metadata-plugin'); module.exports = { serializer: { customSerializer: createMetadataSerializer(defaultSerializer), }, };Prefer using @rnx-kit/metro-serializer to avoid any metro/src import in your app config as well. Based on learnings.
scripts/hermesc-wrapper.js (4)
7-8: Blocker: hard-coded macOS hermesc; resolve via HERMESC_DIR + cross‑platform fallbacks
Derive _hermesc from the invoking dir (exported by the shell wrapper) and add Linux/Pods fallbacks; fail fast if not found.-const realHermescPath = path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/_hermesc'); +function resolveRealHermescPath() { + const c = []; + if (process.env.HERMESC_DIR) c.push(path.join(process.env.HERMESC_DIR, '_hermesc')); + c.push( + path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/_hermesc'), + path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/linux64-bin/_hermesc'), + path.join(PROJECT_ROOT, 'ios/Pods/hermes-engine/destroot/bin/_hermesc'), + ); + return c.find(p => fs.existsSync(p)); +} +const realHermescPath = resolveRealHermescPath(); +if (!realHermescPath) { + console.error('[Hermesc Wrapper] ❌ Could not locate _hermesc (set HERMESC_DIR or install Hermes).'); + process.exit(1); +} +console.log(`[Hermesc Wrapper] Using hermesc: ${realHermescPath}`);
63-69: Guard against missing/invalid contentHash
slice() on undefined throws; bail out gracefully if hash is absent.- const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8')); - const { contentHash } = hashData; + const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8')); + const { contentHash } = hashData || {}; + if (typeof contentHash !== 'string' || contentHash.length === 0) { + console.warn('[Hermesc Wrapper] ⚠️ Invalid contentHash in bundle-hash.json; skipping.'); + process.exit(0); + return; + } @@ - console.log(`[Hermesc Wrapper] Hash: ${contentHash.slice(0, 16)}...`); + console.log(`[Hermesc Wrapper] Hash: ${contentHash.slice(0, 16)}...`);
70-86: Make metadata appends idempotent (strip existing RNUPDATE footer first)
Repeated runs currently re-append and bloat the HBC. Strip an existing footer if present before appending.- const hbcBuffer = fs.readFileSync(hbcFilePath); + let hbcBuffer = fs.readFileSync(hbcFilePath); @@ - const finalBuffer = Buffer.concat([ - hbcBuffer, + function stripRNUpdateFooter(buf, MAGIC) { + const m = MAGIC.length; + if (buf.length < (m * 2 + 4)) return { base: buf, stripped: false }; + const endMagicStart = buf.length - m; + if (!buf.slice(endMagicStart).equals(MAGIC)) return { base: buf, stripped: false }; + const lenStart = endMagicStart - 4; + if (lenStart < 0) return { base: buf, stripped: false }; + const jsonLen = buf.readUInt32LE(lenStart); + const startMagicStart = lenStart - jsonLen - m; + if (startMagicStart < 0) return { base: buf, stripped: false }; + if (!buf.slice(startMagicStart, startMagicStart + m).equals(MAGIC)) return { base: buf, stripped: false }; + return { base: buf.slice(0, startMagicStart), stripped: true }; + } + const { base: baseBuffer } = stripRNUpdateFooter(hbcBuffer, MAGIC); + + const finalBuffer = Buffer.concat([ + baseBuffer, MAGIC, jsonBuffer, lengthBuffer, MAGIC, ]);
91-92: Misleading log: no cleanup actually happens
Remove or implement opt‑in cleanup with an env flag.- console.log(`[Hermesc Wrapper] 🧹 Cleaning up hash file...`); + // Optionally remove hash file if requested: + // if (process.env.RN_UPDATE_CLEAN_HASH === '1') try { fs.unlinkSync(hashFilePath); } catch {}
🧹 Nitpick comments (8)
scripts/bundle-metadata-plugin.js (5)
13-19: Don’t leak globals; attach metadata on globalThis to avoid collisions
Using var BUNDLE_METADATA in the global scope risks clashes. Prefer a single well-namespaced property.-function generateMetadataInjection(contentHash) { - return `// Auto-injected bundle metadata by Metro plugin - var __BUNDLE_METADATA__ = { - contentHash: '${contentHash}' - }; -`; -} +function generateMetadataInjection(contentHash) { + return `// Auto-injected bundle metadata by Metro plugin +(function(g){g.__RN_UPDATE__=Object.assign({},g.__RN_UPDATE__,{contentHash:'${contentHash}'})})(globalThis||global);`; +}
170-176: Use atomic write for bundle-hash.json to avoid torn writes
Interrupted writes can leave a truncated JSON and break the wrapper.- fs.writeFileSync(hashFilePath, JSON.stringify(hashData, null, 2)); + const tmp = `${hashFilePath}.tmp`; + fs.writeFileSync(tmp, JSON.stringify(hashData, null, 2)); + fs.renameSync(tmp, hashFilePath);
5-5: Gate noisy logs behind an env toggle
Project root logs on every bundle can be noisy in CI.-console.log(`[Bundle Metadata] Project root: ${PROJECT_ROOT}`); +if (process.env.RN_UPDATE_DEBUG) { + console.log(`[Bundle Metadata] Project root: ${PROJECT_ROOT}`); +}
167-169: Consider per-build hash file path to avoid races
bundle-hash.json at repo root can be clobbered by concurrent bundles (iOS/Android, debug/release). Parameterize via env (e.g., RN_UPDATE_HASH_FILE) and default to a temp dir.
115-155: Idempotent wrapper setup and error surface
Currently failures are logged but bundling proceeds; consider surfacing a non-zero code or clear signal when all locations fail, so teams catch misconfiguration early.Would you like a guard that throws if successCount === 0?
scripts/hermesc-wrapper.js (3)
31-43: Avoid fixed 500ms delay; poll for output readiness with timeout
Use a short poll for the -out file existence/size instead of a blind sleep to reduce flakiness.- if (code === 0 && isCompileOperation && outputFile) { - console.log(`[Hermesc Wrapper] 🔄 Post-processing HBC file: ${outputFile}`); - - setTimeout(() => { - processHBCFile(outputFile); - }, 500); + if (code === 0 && isCompileOperation && outputFile) { + console.log(`[Hermesc Wrapper] 🔄 Post-processing HBC file: ${outputFile}`); + const start = Date.now(); + const waitUntil = start + 5000; // 5s max + const iv = setInterval(() => { + if (fs.existsSync(outputFile) || Date.now() > waitUntil) { + clearInterval(iv); + processHBCFile(outputFile); + } + }, 100);
1-6: Preflight: ensure hermesc exists before spawn
Fail early with a clearer message if the path is missing (covered by resolveRealHermescPath, but add exists check if you keep the old approach).
45-61: Exit codes and resilience
On missing hash/HBC you exit 0, which can mask misconfig in CI. Consider gating this behavior behind RN_UPDATE_ALLOW_MISSING=1 and otherwise exit non‑zero.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/bundle-metadata-plugin.js(1 hunks)scripts/hermesc-wrapper.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
scripts/bundle-metadata-plugin.js (1)
scripts/hermesc-wrapper.js (7)
require(3-3)fs(5-5)path(4-4)PROJECT_ROOT(6-6)hashFilePath(47-47)hashData(64-64)hashData(65-65)
scripts/hermesc-wrapper.js (1)
scripts/bundle-metadata-plugin.js (6)
path(3-3)fs(2-2)PROJECT_ROOT(4-4)hashFilePath(168-168)hashData(171-174)contentHash(163-163)
| try { | ||
| console.log(`🔧 [Hermesc Setup] Setting up hermesc wrapper for ${locationName}...`); | ||
|
|
||
| fs.renameSync(hermescPath, backupHermescPath); | ||
| console.log(`✅ [Hermesc Setup] ${locationName}: Renamed hermesc -> _hermesc`); | ||
|
|
There was a problem hiding this comment.
Symlink/permissions-safe swap when wrapping hermesc
Pods binaries can be symlinks; rename may fail or break subsequent installs. Prefer copy+rename fallback and detect symlinks before swapping.
try {
console.log(`🔧 [Hermesc Setup] Setting up hermesc wrapper for ${locationName}...`);
- fs.renameSync(hermescPath, backupHermescPath);
+ const stat = fs.lstatSync(hermescPath);
+ if (stat.isSymbolicLink()) {
+ // Preserve original target, copy real file as _hermesc and replace link with wrapper
+ const real = fs.realpathSync(hermescPath);
+ fs.copyFileSync(real, backupHermescPath);
+ fs.unlinkSync(hermescPath);
+ } else {
+ fs.renameSync(hermescPath, backupHermescPath);
+ }
console.log(`✅ [Hermesc Setup] ${locationName}: Renamed hermesc -> _hermesc`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| console.log(`🔧 [Hermesc Setup] Setting up hermesc wrapper for ${locationName}...`); | |
| fs.renameSync(hermescPath, backupHermescPath); | |
| console.log(`✅ [Hermesc Setup] ${locationName}: Renamed hermesc -> _hermesc`); | |
| try { | |
| console.log(`🔧 [Hermesc Setup] Setting up hermesc wrapper for ${locationName}...`); | |
| const stat = fs.lstatSync(hermescPath); | |
| if (stat.isSymbolicLink()) { | |
| // Preserve original target, copy real file as _hermesc and replace link with wrapper | |
| const real = fs.realpathSync(hermescPath); | |
| fs.copyFileSync(real, backupHermescPath); | |
| fs.unlinkSync(hermescPath); | |
| } else { | |
| fs.renameSync(hermescPath, backupHermescPath); | |
| } | |
| console.log(`✅ [Hermesc Setup] ${locationName}: Renamed hermesc -> _hermesc`); |
🤖 Prompt for AI Agents
In scripts/bundle-metadata-plugin.js around lines 47-52, the current code uses
fs.renameSync to swap hermesc which will fail or break when hermesc is a
symlink; detect symlinks with fs.lstatSync(hermescPath).isSymbolicLink() and
handle them specially, and also add a rename-fallback that copies instead of
renaming. Concretely: if hermesc is a symlink, resolve the real file
(fs.realpathSync or fs.readlinkSync + path.resolve), copy the real target to
backupHermescPath (fs.copyFileSync) and preserve permissions (fs.chmodSync with
mode from fs.statSync), then replace hermescPath with your wrapper (write new
file or create a symlink to the wrapper target as appropriate). If not a
symlink, try fs.renameSync as before but catch errors and fall back to
copy+chmod to create the backup file so installs won’t break.
scripts/bundle-metadata-plugin.js
Outdated
| // 使用项目根目录来定位 hermesc | ||
| const hermescLocations = [ | ||
| { | ||
| path: path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/hermesc'), | ||
| name: 'Node Modules' | ||
| }, | ||
| { | ||
| path: path.join(PROJECT_ROOT, 'ios/Pods/hermes-engine/destroot/bin/hermesc'), | ||
| name: 'iOS Pods' | ||
| } | ||
| ]; |
There was a problem hiding this comment.
Add Linux hermesc path (and optionally Windows) to wrapper setup locations
Currently macOS node_modules and iOS Pods only; Linux builds won't be wrapped. Add linux64-bin (and consider win64).
const hermescLocations = [
{
path: path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/hermesc'),
name: 'Node Modules'
},
+ {
+ path: path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/linux64-bin/hermesc'),
+ name: 'Node Modules (Linux)'
+ },
{
path: path.join(PROJECT_ROOT, 'ios/Pods/hermes-engine/destroot/bin/hermesc'),
name: 'iOS Pods'
}
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 使用项目根目录来定位 hermesc | |
| const hermescLocations = [ | |
| { | |
| path: path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/hermesc'), | |
| name: 'Node Modules' | |
| }, | |
| { | |
| path: path.join(PROJECT_ROOT, 'ios/Pods/hermes-engine/destroot/bin/hermesc'), | |
| name: 'iOS Pods' | |
| } | |
| ]; | |
| // 使用项目根目录来定位 hermesc | |
| const hermescLocations = [ | |
| { | |
| path: path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/osx-bin/hermesc'), | |
| name: 'Node Modules' | |
| }, | |
| { | |
| path: path.join(PROJECT_ROOT, 'node_modules/react-native/sdks/hermesc/linux64-bin/hermesc'), | |
| name: 'Node Modules (Linux)' | |
| }, | |
| { | |
| path: path.join(PROJECT_ROOT, 'ios/Pods/hermes-engine/destroot/bin/hermesc'), | |
| name: 'iOS Pods' | |
| } | |
| ]; |
🤖 Prompt for AI Agents
In scripts/bundle-metadata-plugin.js around lines 131 to 141, the
hermescLocations array only includes macOS node_modules and iOS Pods paths so
Linux (and optionally Windows) hermesc binaries won't be found; add entries for
the Linux hermesc path (path.join(PROJECT_ROOT,
'node_modules/react-native/sdks/hermesc/linux64-bin/hermesc') with a name like
'Node Modules (Linux)') and optionally a Windows entry (path.join(PROJECT_ROOT,
'node_modules/react-native/sdks/hermesc/win64-bin/hermesc.exe') named 'Node
Modules (Windows)') and similarly consider adding linux and windows variants for
the ios/Pods location if needed; keep the same object shape (path and name) to
match the existing array.
scripts/hermesc-wrapper.js
Outdated
| // hash 文件在项目根目录 | ||
| const hashFilePath = path.join(PROJECT_ROOT, 'bundle-hash.json'); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Track bundle-hash.json in .gitignore
Avoid committing ephemeral hash files. Add to repo root .gitignore if not already ignored.
🏁 Script executed:
#!/bin/bash
# Verify bundle-hash.json is ignored
{ rg -n '^bundle-hash\.json$' -g .gitignore -S || true; } | sed -n '1,5p'Length of output: 0
🏁 Script executed:
#!/bin/bash
# 1. Check if .gitignore exists at repo root
echo "=== Check .gitignore existence ==="
[ -f .gitignore ] && echo ".gitignore exists" || echo ".gitignore does NOT exist"
# 2. View .gitignore contents
echo -e "\n=== .gitignore contents ==="
cat .gitignore 2>/dev/null | head -50
# 3. Check if bundle-hash.json appears anywhere in .gitignore (flexible matching)
echo -e "\n=== Search for bundle-hash in .gitignore ==="
grep -i "bundle-hash" .gitignore 2>/dev/null || echo "bundle-hash not found in .gitignore"
# 4. View the actual file mentioned in review
echo -e "\n=== View scripts/hermesc-wrapper.js around lines 46-48 ==="
sed -n '40,55p' scripts/hermesc-wrapper.jsLength of output: 1165
Add bundle-hash.json to .gitignore
Verification confirms bundle-hash.json is not currently tracked in .gitignore. Since this file is generated at runtime as an ephemeral artifact (line 47 in scripts/hermesc-wrapper.js), it should be added to .gitignore at the repository root to prevent accidental commits.
🤖 Prompt for AI Agents
In scripts/hermesc-wrapper.js around lines 46 to 48, the runtime-generated file
bundle-hash.json (created at PROJECT_ROOT) is not ignored; update the repository
root .gitignore to add a top-level entry "bundle-hash.json" so the file is not
accidentally committed, then stage and commit the .gitignore change.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
scripts/hermesc-wrapper.js (3)
7-8: Critical: Hard-coded macOS path breaks cross-platform compatibility.This issue was previously flagged but remains unaddressed. The hard-coded
osx-binpath will fail on Linux, Windows, and iOS Pods configurations. The wrapper must dynamically resolve the hermesc binary based on the platform and installation location.
62-87: Critical: Missing idempotency check and contentHash validation.This issue was previously flagged but remains unaddressed. The metadata append logic:
- Does not validate that
contentHashis a non-empty string before using it- Does not check for existing RNUPDATE footers, causing bloat on repeated runs
- Will crash on line 67 if
contentHashis undefined
89-92: Misleading log: no cleanup is performed.This issue was previously flagged but remains unaddressed. The log message "🧹 Cleaning up hash file..." implies that
bundle-hash.jsonis deleted, but no cleanup actually occurs. Either remove the misleading message or implement the cleanup.
🧹 Nitpick comments (2)
scripts/hermesc-wrapper.js (2)
52-52: Remove unreachable return statements.The
returnstatements afterprocess.exit(0)are unreachable and serve no purpose.console.warn(`[Hermesc Wrapper] Skipping metadata injection.`); process.exit(0); - return; } if (!fs.existsSync(hbcFilePath)) { console.warn(`[Hermesc Wrapper] ⚠️ HBC file not found: ${hbcFilePath}`); console.warn(`[Hermesc Wrapper] Skipping metadata injection.`); process.exit(0); - return; }Also applies to: 59-59
74-74: Extract magic string to a module-level constant.The "RNUPDATE" magic string is hard-coded and appears multiple times. Defining it as a constant improves maintainability and makes the format more discoverable.
+const MAGIC = Buffer.from('RNUPDATE', 'utf8'); + const args = process.argv.slice(2); ... function processHBCFile(hbcFilePath) { ... try { ... - const MAGIC = Buffer.from('RNUPDATE', 'utf8'); const jsonBuffer = Buffer.from(metadataJson, 'utf8');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/bundle-metadata-plugin.js(1 hunks)scripts/hermesc-wrapper.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/bundle-metadata-plugin.js
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/hermesc-wrapper.js (1)
scripts/bundle-metadata-plugin.js (6)
path(3-3)fs(2-2)PROJECT_ROOT(4-4)hashFilePath(165-165)hashData(168-171)contentHash(162-162)
🔇 Additional comments (3)
scripts/hermesc-wrapper.js (3)
10-19: LGTM: Argument parsing logic is sound.The detection of compile operations via
-emit-binaryand output file extraction via-outis correctly implemented.
26-29: LGTM: Error handling is appropriate.Properly logs spawn failures and exits with non-zero code.
48-60: Verify that exit code 0 is appropriate for missing files.When
bundle-hash.jsonor the HBC file is missing, the wrapper exits with code 0 (success). This may hide legitimate build errors. If these files are expected to exist after a successful hermesc compile, the exit code should be 1 to signal failure.Clarify the expected behavior:
- Are there valid scenarios where these files won't exist after a successful compile?
- Should missing files be treated as warnings (exit 0) or errors (exit 1)?
If these are always errors, apply this diff:
if (!fs.existsSync(hashFilePath)) { console.warn(`[Hermesc Wrapper] ⚠️ Hash file not found: ${hashFilePath}`); console.warn(`[Hermesc Wrapper] Skipping metadata injection.`); - process.exit(0); + process.exit(1); return; } if (!fs.existsSync(hbcFilePath)) { console.warn(`[Hermesc Wrapper] ⚠️ HBC file not found: ${hbcFilePath}`); console.warn(`[Hermesc Wrapper] Skipping metadata injection.`); - process.exit(0); + process.exit(1); return; }
| setTimeout(() => { | ||
| processHBCFile(outputFile); | ||
| }, 500); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove the 500ms setTimeout workaround.
The arbitrary delay suggests a race condition or file system synchronization issue that should be resolved properly rather than masked with a timeout. This is brittle and may fail under load or on slower systems.
Consider one of these approaches:
Option 1: Synchronous waiting (if hermesc writes are buffered)
- setTimeout(() => {
- processHBCFile(outputFile);
- }, 500);
+ processHBCFile(outputFile);Option 2: Polling with retry (if writes are truly async)
- setTimeout(() => {
- processHBCFile(outputFile);
- }, 500);
+ waitForFileStable(outputFile, (err) => {
+ if (err) {
+ console.error(`[Hermesc Wrapper] ❌ Output file not ready:`, err);
+ process.exit(1);
+ }
+ processHBCFile(outputFile);
+ });
+
+function waitForFileStable(filePath, callback, maxRetries = 10, delay = 100) {
+ let lastSize = -1;
+ let retries = 0;
+
+ const check = () => {
+ try {
+ const stats = fs.statSync(filePath);
+ if (stats.size === lastSize) {
+ callback(null);
+ } else {
+ lastSize = stats.size;
+ retries++;
+ if (retries >= maxRetries) {
+ callback(new Error('File not stable'));
+ } else {
+ setTimeout(check, delay);
+ }
+ }
+ } catch (err) {
+ callback(err);
+ }
+ };
+ check();
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setTimeout(() => { | |
| processHBCFile(outputFile); | |
| }, 500); | |
| processHBCFile(outputFile); |
🤖 Prompt for AI Agents
In scripts/hermesc-wrapper.js around lines 37 to 39, remove the brittle
setTimeout(…,500) workaround and replace it with a deterministic wait: if the
hermesc write exposes a completion callback or stream, invoke
processHBCFile(outputFile) from that completion handler; otherwise implement a
short polling/retry loop using fs.promises.stat or fs.promises.access to confirm
the file exists and its size is stable (repeat check with small delays and a max
timeout/backoff), then call processHBCFile once the file is present and
unchanged; ensure errors/timeouts are handled and surfaced rather than relying
on a fixed sleep.
| function processHBCFile(hbcFilePath) { | ||
| const hashFilePath = path.join(PROJECT_ROOT, 'bundle-hash.json'); | ||
|
|
||
| if (!fs.existsSync(hashFilePath)) { | ||
| console.warn(`[Hermesc Wrapper] ⚠️ Hash file not found: ${hashFilePath}`); | ||
| console.warn(`[Hermesc Wrapper] Skipping metadata injection.`); | ||
| process.exit(0); | ||
| return; | ||
| } | ||
|
|
||
| if (!fs.existsSync(hbcFilePath)) { | ||
| console.warn(`[Hermesc Wrapper] ⚠️ HBC file not found: ${hbcFilePath}`); | ||
| console.warn(`[Hermesc Wrapper] Skipping metadata injection.`); | ||
| process.exit(0); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const hashData = JSON.parse(fs.readFileSync(hashFilePath, 'utf8')); | ||
| const { contentHash } = hashData; | ||
|
|
||
| console.log(`[Hermesc Wrapper] 📝 Injecting metadata into HBC...`); | ||
| console.log(`[Hermesc Wrapper] Hash: ${contentHash.slice(0, 16)}...`); | ||
|
|
||
| const hbcBuffer = fs.readFileSync(hbcFilePath); | ||
|
|
||
| const metadata = { contentHash }; | ||
| const metadataJson = JSON.stringify(metadata); | ||
|
|
||
| const MAGIC = Buffer.from('RNUPDATE', 'utf8'); | ||
| const jsonBuffer = Buffer.from(metadataJson, 'utf8'); | ||
| const lengthBuffer = Buffer.alloc(4); | ||
| lengthBuffer.writeUInt32LE(jsonBuffer.length); | ||
|
|
||
| const finalBuffer = Buffer.concat([ | ||
| hbcBuffer, | ||
| MAGIC, | ||
| jsonBuffer, | ||
| lengthBuffer, | ||
| MAGIC, | ||
| ]); | ||
|
|
||
| fs.writeFileSync(hbcFilePath, finalBuffer); | ||
|
|
||
| console.log(`[Hermesc Wrapper] ✅ Successfully injected metadata into: ${hbcFilePath}`); | ||
| console.log(`[Hermesc Wrapper] 🧹 Cleaning up hash file...`); | ||
|
|
||
| process.exit(0); | ||
| } catch (error) { | ||
| console.error(`[Hermesc Wrapper] ❌ Failed to process HBC file:`, error); | ||
| process.exit(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
Race condition: parallel builds will corrupt bundle-hash.json.
If multiple hermesc processes compile in parallel (common in multi-platform or multi-architecture builds), they will all attempt to read/write the same bundle-hash.json at the project root. This can cause:
- Read of stale or incorrect hash data
- Writes interleaved or overwritten
- Metadata injected into the wrong HBC file
Consider one of these solutions:
Option 1: Use per-output hash files
- const hashFilePath = path.join(PROJECT_ROOT, 'bundle-hash.json');
+ const hashFilePath = hbcFilePath.replace(/\.hbc$/, '.hash.json');Then update bundle-metadata-plugin.js to write hash files adjacent to each output.
Option 2: Add process locking
Use a locking library like proper-lockfile to ensure exclusive access to bundle-hash.json.
Option 3: Pass hash via environment variable
Have the plugin set an environment variable with the hash, avoiding the file altogether:
- const hashFilePath = path.join(PROJECT_ROOT, 'bundle-hash.json');
- if (!fs.existsSync(hashFilePath)) {
- console.warn(`[Hermesc Wrapper] ⚠️ Hash file not found: ${hashFilePath}`);
+ const contentHash = process.env.RNUPDATE_CONTENT_HASH;
+ if (!contentHash) {
+ console.warn(`[Hermesc Wrapper] ⚠️ RNUPDATE_CONTENT_HASH not set`);
console.warn(`[Hermesc Wrapper] Skipping metadata injection.`);
process.exit(0);
- return;
}Committable suggestion skipped: line range outside the PR's diff.
| }); | ||
|
|
||
| function processHBCFile(hbcFilePath) { | ||
| const hashFilePath = path.join(PROJECT_ROOT, 'bundle-hash.json'); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify bundle-hash.json is in .gitignore.
The bundle-hash.json file is a runtime-generated artifact that should not be committed to version control. A previous review flagged this, but it's unclear if it was addressed.
🏁 Script executed:
#!/bin/bash
# Verify bundle-hash.json is ignored
if [ -f .gitignore ]; then
if grep -q "bundle-hash\.json" .gitignore; then
echo "✅ bundle-hash.json is in .gitignore"
else
echo "❌ bundle-hash.json is NOT in .gitignore"
fi
else
echo "⚠️ No .gitignore file found at repository root"
fi
# Show .gitignore contents for context
echo -e "\n=== Current .gitignore ==="
cat .gitignore 2>/dev/null || echo "File not found"Length of output: 932
🏁 Script executed:
#!/bin/bash
# Check for nested .gitignore files that might ignore bundle-hash.json
echo "=== Searching for nested .gitignore files ==="
find . -name ".gitignore" -type f 2>/dev/null | head -20
# Search for bundle-hash.json references in codebase
echo -e "\n=== References to bundle-hash.json ==="
rg "bundle-hash\.json" --type-list | head -5
rg "bundle-hash" -n -B2 -A2 2>/dev/null || echo "No additional references found"Length of output: 1539
🏁 Script executed:
#!/bin/bash
# Examine context around bundle-hash.json usage in hermesc-wrapper.js
echo "=== hermesc-wrapper.js context (lines 40-60) ==="
sed -n '40,60p' git/scripts/hermesc-wrapper.js
echo -e "\n=== bundle-metadata-plugin.js context (lines 160-175) ==="
sed -n '160,175p' git/scripts/bundle-metadata-plugin.jsLength of output: 262
🏁 Script executed:
#!/bin/bash
# Examine context around bundle-hash.json usage - corrected paths
echo "=== hermesc-wrapper.js context (lines 40-60) ==="
sed -n '40,60p' scripts/hermesc-wrapper.js 2>/dev/null || echo "File not found at scripts/hermesc-wrapper.js"
echo -e "\n=== bundle-metadata-plugin.js context (lines 160-175) ==="
sed -n '160,175p' scripts/bundle-metadata-plugin.js 2>/dev/null || echo "File not found"
# Also try finding the files
echo -e "\n=== Locating files ==="
find . -name "hermesc-wrapper.js" -o -name "bundle-metadata-plugin.js" 2>/dev/null | head -10Length of output: 1495
Add bundle-hash.json to .gitignore at repository root.
The file is a runtime-generated artifact created by bundle-metadata-plugin.js (line 171: fs.writeFileSync()) and must not be committed to version control. Currently it is missing from .gitignore.
Action: Add bundle-hash.json to the root .gitignore file.
🤖 Prompt for AI Agents
In scripts/hermesc-wrapper.js around line 46, the runtime-generated file
bundle-hash.json (created by bundle-metadata-plugin.js) is not ignored; add
"bundle-hash.json" as a new entry to the repository root .gitignore file so the
generated artifact is not committed to version control.
1 participant
Summary by CodeRabbit