BabylonJS / BabylonJS/JsRuntimeHost
Hermes: two upstream conformance gaps worked around in the unit tests (for...in shadowing, napi ToObject)
- Dominant language
- C++
- Stars
- 22
- Forks
- 23
- Avg merge
- 3d 14h
- Merged PRs (30d)
- 6
Description
[Filed by Copilot on behalf of @bghgary]
Tracks two **distinct** Hermes conformance gaps that #218 works around in `Tests/UnitTests/Scripts/tests.ts`. They are in different layers of Hermes and neither is fixable here — this issue exists so the workarounds have a referent and can be removed when upstream fixes them.
Hermes is pinned in the top-level `CMakeLists.txt` to `facebook/hermes` `static_h` at `348582831f50954895da8e80cc91112d51036c69` (tip as of 2026-06-03).
## 1. `for...in` reports a property shadowed by a non-enumerable own property
**Layer:** Hermes VM (`lib/VM/JSObject.cpp`)
Per spec, `for...in` adds every own key to its visited set *regardless of enumerability*, but only emits the enumerable ones. A non-enumerable own property must therefore hide a same-named enumerable inherited one.
```js
const parent = { shared: 1 };
const child = Object.create(parent);
Object.defineProperty(child, "shared", { value: 2, enumerable: false });
const keys = [];
for (const k in child) { keys.push(k); }
// expected: [] (child.shared is 2; the inherited 1 is unreachable)
// Hermes: ["shared"]
```
Multi-level, same cause:
```js
const gp = { deep: 0 };
const mid = Object.create(gp);
mid.middle = 1;
Object.defineProperty(mid, "deep", { value: 9, enumerable: false });
const leaf = Object.create(mid);
leaf.own = 3;
// expected: ["own", "middle"]
// Hermes: ["own", "middle", "deep"]
```
Plain inheritance with no shadowing behaves correctly, so this is specific to the shadowing rule rather than enumeration generally.
**Cause**, from `appendAllPropertyKeys` in `lib/VM/JSObject.cpp` at the pinned SHA: own keys are collected with `OwnKeysFlags().plusIncludeNonSymbols().plusKeepSymbols()` — no `IncludeNonEnumerable` — so `enumerableProps` holds only enumerable own properties. The dedup set is populated exclusively by `addToDedup`, which is called only after a property has been pushed. A non-enumerable own property is therefore never collected and never deduped, so the inherited one is emitted.
Notably Hermes' **own Node-API** implementation gets this right, and deliberately — `API/napi/hermes_napi_object.cpp` sets `okFlags.plusIncludeNonEnumerable()` with the comment "Always include non-enumerable so we can post-filter uniformly", and inserts into `seenKeys` *before* applying the enumerable filter. So the two enumeration paths inside Hermes disagree with each other.
**Impact here:** none on our behaviour — `napi_get_property_names` is correct on Hermes. It only means Hermes' `for...in` cannot be used as a test oracle. #218 handles this with a runtime probe (`forInHonoursShadowing`) rather than naming the engine, so it self-heals if upstream fixes it.
**Evidence:** source inspection at the pinned SHA, plus an empirical repro on the prebuilt Hermes CLI **v0.13.0**, which reproduces both cases above (V8 is correct on both). See the caveat below.
## 2. `napi_get_property_names` rejects primitives instead of applying `ToObject`
**Layer:** Hermes Node-API (`API/napi/hermes_napi_object.cpp`)
Node-API specifies the argument is coerced with `ToObject`, so a primitive is wrapped and its properties reported — `napi_get_property_names("ab")` should yield `["0", "1"]`. V8 does this via `CHECK_TO_OBJECT`.
Hermes' `napi_get_all_property_names` (which `napi_get_property_names` delegates to) instead does:
```cpp
RETURN_STATUS_IF_FALSE(
env, objPhv != nullptr && objPhv->isObject(), napi_object_expected);
```
so any non-object is rejected outright with no coercion path.
**Impact here:** the primitive-wrapping cases in #218's `argument coercion` block are skipped on Hermes.
**Evidence:** source inspection at the pinned SHA only — see below.
## Before filing upstream
`facebook/hermes` is actively maintained and responsive, but two recent issues were closed as suspected AI-generated spam specifically for lacking a minimal repro, and maintainers routinely distinguish between `stable`, `static_h` and V1 when triaging. So neither of these should be filed upstream until:
- **Issue 1** is reproduced against the **pinned `static_h` SHA**, not just v0.13.0. The current repro is roughly two years older than the pin; inspection at the pin shows the same defect, but that is not the same as running it.
- **Issue 2** has *any* runnable repro. It is a Node-API entry point and is not reachable from the `hermes.exe` CLI, so there is currently no empirical evidence at all.
Building this repo's Hermes leg locally satisfies both: it uses the exact pinned SHA and exposes the Node-API surface.
## Removing the workarounds
Both live in `Tests/UnitTests/Scripts/tests.ts`:
- **Issue 1** — the `forInHonoursShadowing` probe. Nothing to do; it starts using `for...in` as an oracle again once upstream is fixed.
- **Issue 2** — the `napiEngine === "Hermes"` skip on the primitive cases. Un-skip once upstream applies `ToObject`.
Contributor guide
Assessment
This issue has not been assessed yet.