hermes: Lacking support for ES6 block scoping

Bug Description

There is a bug in Object.defineProperty that always returns the last set property regardless of what property is being accessed.

  • I have run gradle clean and confirmed this bug does not occur with JSC

Hermes version: 0.8.1 React Native version: n/a OS version: n/a Platform: arm64-v8a, x86_64

Steps To Reproduce

Run the following code example with hermes-engine-cli and node, and compare the output:

const target = {
  a: "a",
  b: "b",
  c: "c",
};

const copy = {};
let keys = "";
for (const prop in target) {
  keys += " " + prop;
  Object.defineProperty(copy, prop, {
    get: () => target[prop],
    enumerable: true,
  });
}

// I don't know how to output to console so I'm just using a throw 😛
throw new Error(`copy: a = ${copy.a}, b = ${copy.b}, c = ${copy.c} (keys:${keys})`);

Output:

Uncaught Error: copy: a = c, b = c, c = c (keys: a b c)
    at global (repro.js:18:16)

The Expected Behavior

This should throw copy: a = a, b = b, c = c, as Node does:

Error: copy: a = a, b = b, c = c (keys: a b c)
    at Object.<anonymous> (/~/repro.js:18:7)
    at Module._compile (internal/modules/cjs/loader.js:1072:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1101:10)
    at Module.load (internal/modules/cjs/loader.js:937:32)
    at Function.Module._load (internal/modules/cjs/loader.js:778:12)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:76:12)
    at internal/main/run_main_module.js:17:47

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 10
  • Comments: 27 (10 by maintainers)

Most upvoted comments

Hi all,

Indeed hermes’ current behavior w.r.t. block scoping is pretty surprising – it happily accepts const and let, but doesn’t honor the semantics around them – captures, constness, etc.

As @tmikov has been mentioning, this is a fairly invasive change, and we’ve been trying to figure out a strategy to complete the work in the least disruptive way possible. Ideally, any ES5-lowered code wouldn’t be affected, at all, by the block scope-related changes.

I have been working on this change for a while, and we are optimistic about being able to finally merge the work. You may notice that I merged over 20 diffs today – those are all related to the block scope work (removing some assumptions, refactoring other parts of the code etc). Unfortunately, there are still a fair bit to be merged, so it could be a while until Hermes has block scoping support.

Hermes will support block scoping in the next major revision. Since EcmaScript compatibility is tracked separately, closing this issue.

@jpporto Thanks for merging above. Any update on when would this feature land?

Would be great to switch for ESBuild for metro

@kelset some time in 2024. I wish I could be more specific.

To be more precise, it is not paused, but it is targeting the next major release of Hermes, not the next release of RN. The distinction is subtle but important.

This is not related to Object.defineProperty(). Hermes does not fully support ES6 or ES6 block scoping in particular (nor does it claim to). FWIW, we don’t support const either (it is treated as an alias of var), but that does not affect this example.

You need to run the source through the regular Babel pipeline, resulting in the following, which does execute correctly:

var target = {
  a: "a",
  b: "b",
  c: "c"
};
var copy = {};
var keys = "";

var _loop = function _loop(prop) {
  keys += " " + prop;
  Object.defineProperty(copy, prop, {
    get: function get() {
      return target[prop];
    },
    enumerable: true
  });
};

for (var prop in target) {
  _loop(prop);
}

FWIW, we have had ES6 block scoping in the works for a very long time, but we are finding it difficult to integrate since the changes are pervasive and it is very disruptive to the entire project (compiler, interpreter, debugger, affects almost every single compiler test, etc).

@tmikov any movement on this over the past year? This feature would be greatly appreciated by a lot of people I think trying to use alternative bundlers to work around the limitations of metro.

To be more precise, it is not paused, but it is targeting the next major release of Hermes, not the next release of RN. The distinction is subtle but important.

~~Oh I’m hearing conflicting information then. Can you clarify this comment here: https://docs.google.com/document/d/1Q_kbafCLBUfe89L5GOysVzozt7_jdmpKhkYqHLVIx0g/edit?disco=AAAA1vGMOto~~

See updated comment here: https://github.com/facebook/hermes/issues/575#issuecomment-1658812284

This work was paused by the Hermes team and will not be in 0.73

Edit: Clarification after speaking to @tmikov, there is active work on this feature but will not be available in the React Native 0.73 release. Just to close up any confusion! Sorry!

makes sense - no worries, we’ll check back around when 0.73 will be cut to see how’s it going on this end.

same bug

function myFunction(arg){
  switch(arg) {
    case "1":{
      function onClick(){
        print("onClick 1")          
      }
      onClick()
      print("1")
      break;
    }
    case "2":{
       function onClick(){
          print("onClick 2")          
        }
        onClick()
        print("2")
    break;
    }
  }
}

myFunction("1");

Expected output ‘onClick 1’ but it’s ‘onClick 2’ image

https://hermesengine.dev/playground/

I will come to migrate the big react native project to hermes engine how can know all my functions work as well?

my app work with typeScript

Config my tsconfig.json target property to ES5 then fix all typeScript errors Does this ensure that everything will work as expected?

hey @ljharb could I just ask you to rewrite your comment

Hermes’ behavior is how for-in and for loops behave, but i believe for-of loops specifically address this (ie, the issue is correct)

in a bit of a more verbose/n00b-friendly fashion? I’ve read it I think 5 times now but I still don’t understand what you are saying 😓