Lacking support for ES6 block scoping

[tido64]

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

[tido64]

cc @Huxpro, @mganandraj, @JasonVMo

[ljharb]

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

[kelset]

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 😓

[tmikov]

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);
}

[tido64]

Thanks for the explanation and workaround, @tmikov. That pushed me in the right direction. 😄

[tmikov]

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).

[evelant]

@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.

[Bardiamist]

OMG in my case the variable was got absolutelly from another block. Now the app has unexpected behavior in the production.

Something like this with redux-saga:

function* run({ a }) {
  yield call(a);
}

if (true) {
  function* a() { console.log('1'); }
  yield call(run, { a });
} else {
  function* a() { console.log('2'); }
  yield call(run, { a });
}

Expected output '1' but it's '2'

Hope ES6 block scoping will supported soon

[jpporto]

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.

[dlebedynskyi]

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

Would be great to switch for ESBuild for metro

[YOEL311]

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'

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?

[tmikov]

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

[YOEL311]

@tmikov
Thanks
if I use react native 69 version can I get that?

[tmikov]

@YOEL311 when we release the next major version of Hermes, it will probably be source level compatible with RN 69. However I don't think that RN backports new Hermes versions to older version of RN, so you might have to do that yourself.

[YOEL311]

@tmikov
How can I do this myself?

[tmikov]

@YOEL311 what do you want to do yourself?

[YOEL311]

@tmikov
I want to update hermes to order support ES6 (After the next version is released) with my app in react native 69

[kelset]

sidenote: @tmikov so just to be clear, this will be available with RN 73 (the current main branch version) correct?

[tmikov]

@kelset No. Hermes releases are not synchronized with RN versions. We don't have a definitive public timeline for the next major Hermes release yet.

[kelset]

gotcha - thanks for clarifying 👍 once you have an estimated ETA for the next major can you let me / the release crew for RN know so that if it's "around" when we'll plan to cut the 0.73 branch we can make sure to make the two happen sequentially? (first Hermes, then we cut 0.73)

[tmikov]

@kelset I am not really sure about the schedule of RN releases, but my gut feeling is that RN shouldn't wait for the next version of Hermes.

There are major changes coming to Hermes for the next version, like 6-month stable releases, bytecode stability, better ES6 compliance, much better numeric performance, and more. I hope we will be able to start talking more about it soon.

It is indeed a very interesting question whether the 6-month release cycle of Hermes should be synchronized to RN releases, or RN should just pick the current stable Hermes release at any point. We are also seriously thinking about improved ABI stability, which might make this question less important.

[kelset]

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

[lunaleaps]

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!

[tmikov]

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.

[lunaleaps]

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: #575 (comment)

[kelset]

@tmikov do we have any ETAs yet for the next major of Hermes?

[tmikov]

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