Fix metro-runtime's require system getter checks for ESM live bindings

[kitten]

Summary

In Expo, we're about to introduce ESM live bindings. We noticed that the metro-runtime performs checks for getters to skip over for Fast Refresh checks, likely because of CommonJS modules that may be doing module.exports = { get() { throw new Error() } } and the likes.

This check is redundant on ESModule-transpiled modules, since we know that getters that were transpiled in for ESM exports are safe to access.

For example, we may have entries such as:

Object.defineProperty(exports, '__esModule', { value: true });
Object.defineProperty(exports, 'default', {
  enumerable: true,
  get: function() {
    return _default;
  },
});

See: expo/expo#39525

Changelog: [Fix] Fix metro-runtime's fast refresh checks to not skip over getters for ES modules

Test plan

• This can most easily be reproduced against Expo SDK 54, which has a live-bindings suppoting import-export plugin. For repro, see: [SDK 54] Fast Refresh causes remount expo/expo#39505

[facebook-github-bot]

@robhogan has imported this pull request. If you are a Meta employee, you can view this in D82119709.

[robhogan]

Looks good - thanks

There's a small edge case here where a module may export a dangerous getter and be an __esModule, because we allow mixed syntax:

module.exports = {
  get danger() {
    throw 'problems';
  }
};
export default 'spanner';

...but I think we can live with that - nobody should be writing it. You might want to disallow hybrid CJS/ESM syntax on your end to be sure.

[kitten]

Current thinking is to leave it on until we find a library that relies on mixed ESM+CJS that causes this to break 👍 Mostly because we're curious if we need to actively prohibit this

[facebook-github-bot]

@robhogan merged this pull request in 076c3b4.