Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RN/MMM + SES: 20 Promise warnings #13

Closed
leotm opened this issue Mar 6, 2023 · 6 comments
Closed

RN/MMM + SES: 20 Promise warnings #13

leotm opened this issue Mar 6, 2023 · 6 comments
Labels
android iOS jsc JavaScriptCore lockdown promise Promise polyfill v8 Chrome V8 warning build/runtime remaining successful

Comments

@leotm
Copy link
Member

leotm commented Mar 6, 2023

Follow-up to

As seen in


Screenshot 2023-03-01 at 11 54 24

 WARN  Removing intrinsics.Promise._B
 WARN  Removing intrinsics.Promise._C
 WARN  Removing intrinsics.Promise._D
 WARN  Removing intrinsics.Promise.resolve.prototype
 WARN  Tolerating undeletable intrinsics.Promise.resolve.prototype === undefined
 WARN  Removing intrinsics.Promise.all.prototype
 WARN  Tolerating undeletable intrinsics.Promise.all.prototype === undefined
 WARN  Removing intrinsics.Promise.allSettled.prototype
 WARN  Tolerating undeletable intrinsics.Promise.allSettled.prototype === undefined
 WARN  Removing intrinsics.Promise.reject.prototype
 WARN  Tolerating undeletable intrinsics.Promise.reject.prototype === undefined
 WARN  Removing intrinsics.Promise.race.prototype
 WARN  Tolerating undeletable intrinsics.Promise.race.prototype === undefined
 WARN  Removing intrinsics.%PromisePrototype%.then.prototype
 WARN  Tolerating undeletable intrinsics.%PromisePrototype%.then.prototype === undefined
 WARN  Removing intrinsics.%PromisePrototype%.done
 WARN  Removing intrinsics.%PromisePrototype%.finally.prototype
 WARN  Tolerating undeletable intrinsics.%PromisePrototype%.finally.prototype === undefined
 WARN  Removing intrinsics.%PromisePrototype%.catch.prototype
 WARN  Tolerating undeletable intrinsics.%PromisePrototype%.catch.prototype === undefined

Tolerable as we have a fully functioning RN app
These can be ignored via RN's internal LogBox

Or worst-case omit our RN Promise polyfill, then testing meta-maskmobile

Ideal result: Include RN Promise polyfill (or other) with 0 warnings

Not a blocker in RN, unless proves in metamask-mobile (i.e. fuller RN repo's)

First seen in

  • MMM: RN 0.66.5 + SES 0.18.1
@leotm leotm added iOS jsc JavaScriptCore labels Mar 12, 2023
@leotm leotm changed the title RN 0.66.5 + SES 0.18.1 + iOS + JSC: Promise warnings RN 0.66.5 + SES 0.18.1: Promise warnings Mar 12, 2023
@leotm leotm added the warning build/runtime remaining successful label Mar 12, 2023
@leotm leotm changed the title RN 0.66.5 + SES 0.18.1: Promise warnings RN 0.66.5 + SES 0.18.1: 20 Promise warnings Mar 12, 2023
@leotm leotm added the promise Promise polyfill label Mar 12, 2023
@leotm
Copy link
Member Author

leotm commented Mar 22, 2023

nb: from before

a Babel plugin looking most likely (least intrusive/patchy) to switcheroo (transform) back

but first the big why polyfill our Android/iOS JSC/V8 global Promise in the first place
and why is this polyfill nonstandard to SES

@leotm
Copy link
Member Author

leotm commented Mar 24, 2023

following-up above and

we're polyfilling Promise because 8 years ago (diff) since (2015) v0.4.0+

// InitializeJavaScriptAppEngine.js
// ...
function setupPromise() {
  // The native Promise implementation throws the following error:
  // ERROR: Event loop not supported.
  GLOBAL.Promise = require('Promise');
}
// ...

still present in main / 0.72-stable (diff)

// react-native/packages/react-native/Libraries/Core/polyfillPromise.js
// ...
/**
 * Set up Promise. The native Promise implementation throws the following error:
 * ERROR: Event loop not supported.
 *
 * If you don't need these polyfills, don't use InitializeCore; just directly
 * require the modules you need from InitializeCore for setup.
 */
// ...

still the case? after removing our polyfill(s) and testing native Promise methods below

  • Promise.all()
  • Promise.allSettled()
  • Promise.any()
  • Promise.race()
  • Promise.reject()
  • Promise.resolve()
  • Promise.prototype.then()
  • Promise.prototype.catch()
  • Promise.prototype.finally()
  • Promise.prototype.done()
    • rejected (no pun intended) i.e. nonstandard, removed in RN v0.70+

nb: https://test262.report in future

// Promise.all() example
const promise1 = Promise.resolve('Resolved value 1');
const promise2 = Promise.resolve('Resolved value 2');
const promise3 = Promise.resolve('Resolved value 3');

(async () => {
  try {
    const values = await Promise.all([promise1, promise2, promise3]);
    console.log(values); // ["Resolved value 1", "Resolved value 2", "Resolved value 3"]
  } catch (error) {
    console.log(error);
  }
})();

// Promise.allSettled() example
const promise4 = Promise.resolve('Resolved value 4');
const promise5 = Promise.reject('Rejected value 5');

(async () => {
  try {
    const results = await Promise.allSettled([promise4, promise5]);
    console.log(results);
    // [
    //   { status: "fulfilled", value: "Resolved value 4" },
    //   { status: "rejected", reason: "Rejected value 5" }
    // ]
  } catch (error) {
    console.log(error);
  }
})();

// Promise.any() example
const promise6 = Promise.reject('Rejected value 6');
const promise7 = Promise.resolve('Resolved value 7');

(async () => {
  try {
    const value = await Promise.any([promise6, promise7]);
    console.log(value); // "Resolved value 7"
  } catch (errors) {
    console.log(errors);
  }
})();

// Promise.prototype.catch() example
const promise8 = Promise.reject('Rejected value 8');

(async () => {
  promise8.catch((error) => {
    console.log(error); // "Rejected value 8"
  });
})();

// Promise.prototype.finally() example
const promise9 = Promise.resolve('Resolved value 9');

(async () => {
  try {
    const value = await promise9;
    console.log(value); // "Resolved value 9"
  } catch (error) {
    console.log(error);
  } finally {
    console.log('Promise 9 is settled');
  }
})();

// Promise.race() example
const promise10 = new Promise((resolve) => {
  setTimeout(() => {
    resolve('Resolved value 10');
  }, 1000);
});

const promise11 = new Promise((resolve) => {
  setTimeout(() => {
    resolve('Resolved value 11');
  }, 500);
});

(async () => {
  try {
    const value = await Promise.race([promise10, promise11]);
    console.log(value); // "Resolved value 11"
  } catch (error) {
    console.log(error);
  }
})();

// Promise.reject() example
const promise12 = Promise.reject(new Error('Rejected value 12'));

(async () => {
  promise12.catch((error) => {
    console.log(error.message); // "Rejected value 12"
  });
})();

// Promise.resolve() example
const promise13 = Promise.resolve('Resolved value 13');

(async () => {
  const value = await promise13;
  console.log(value); // "Resolved value 13"
})();

// Promise.prototype.then() example
const promise14 = Promise.resolve('Resolved value 14');

(async () => {
  const value = await promise14;
  console.log(value); // "Resolved value 14"
})();
# iOS JSC/V8, runtime ok (V8 doesnt log TypeError)
 LOG  [TypeError: Promise.any is not a function. (In 'Promise.any([promise6, promise7])', 'Promise.any' is undefined)]
 LOG  Rejected value 8
 LOG  Resolved value 9
 LOG  Promise 9 is settled
 LOG  Rejected value 12
 LOG  Resolved value 13
 LOG  Resolved value 14
 LOG  ["Resolved value 1", "Resolved value 2", "Resolved value 3"]
 LOG  [{"status": "fulfilled", "value": "Resolved value 4"}, {"reason": "Rejected value 5", "status": "rejected"}]
 LOG  Running "RN665" with {"rootTag":31,"initialProps":{}}
 LOG  Resolved value 11

# on event e.g. scroll
SES_UNHANDLED_REJECTION: Rejected value 6

nb: yday JSC/V8 throw/console.error'ed and JSC in Metro twice, so smth was cached - now only logging testing in fresh PoC

so we're polyfilling all of

// node_modules/react-native/Libraries/Promise.js
const Promise = require('promise/setimmediate/es6-extensions');

require('promise/setimmediate/done');
require('promise/setimmediate/finally');

if (__DEV__) {
  require('promise/setimmediate/rejection-tracking').enable(
    require('./promiseRejectionTrackingOptions').default,
  );
}

module.exports = Promise;

i.e. overwriting 8 native Promise methods: Promise.resolve, Promise.all, Promise.allSettled, Promise.reject, Promise.race, Promise.prototype.catch (es6-extensions), Promise.prototype.done, Promise.prototype.finally
warned by SES as nonstandard: tolerating undeletable intrinstics

to only polyfill Promise.any :rage1: overkill

i.e. no longer due to the original (8 year old) stale code comment: ERROR: Event loop not supported

@leotm
Copy link
Member Author

leotm commented Mar 24, 2023

action item PR contributions reflecting above and #5 (comment)

opt1

  • then/promise: fix all (SES-detected) nonstandard Promise methods to standard

opt2

  • then/promise: extract Promise.any from es6-extensions like done and finally (or find other lib)
  • facebook/react-native: only polyfill Promise.any then remove others
  • then/promise: standardise Promise.any if nonstandard

opt3

  • RN (4) JS engines: (android/ios, jsc/v8) add/patch standard Promise.any native method to each engine
  • facebook/react-native: update RN core to use above new engine releases then remove polyfills

until resolved

  • endojs/endo: (whitelist) expect nonstandard RN Promise methods

and

  • facebook/react-native: remove old docs ERROR: Event loop not supported no longer true

edit: re-check Promise methods with polyfill(s)

edit: re-check recent changelogs RE Promise.any and Promise.allSettled

@leotm leotm changed the title RN 0.66.5 + SES 0.18.1: 20 Promise warnings [JSC/V8] RN 0.66.5 + SES 0.18.1: 20 Promise warnings Apr 17, 2023
@leotm
Copy link
Member Author

leotm commented Jul 14, 2023

so we're polyfilling all of

...

i.e. overwriting 8 native Promise methods: Promise.resolve, Promise.all, Promise.allSettled, Promise.reject, Promise.race, Promise.prototype.catch (es6-extensions), Promise.prototype.done, Promise.prototype.finally
warned by SES as nonstandard: tolerating undeletable intrinstics

to only polyfill Promise.any :rage1: overkill

i.e. no longer due to the original (8 year old) stale code comment: ERROR: Event loop not supported

reason 2 for excluding: require('./polyfillPromise') or polyfillGlobal('Promise', () => require('../Promise'))
MMM + SES + V8: including it increasing Remote JS Debugging app bootup time ~20-30s
reason to keep it included (default): excluding it causing SES_UNHANDLED_REJECTION error #21

@leotm leotm changed the title [JSC/V8] RN 0.66.5 + SES 0.18.1: 20 Promise warnings [JSC/V8] RN/MMM + SES: 20 Promise warnings Aug 17, 2023
@leotm leotm changed the title [JSC/V8] RN/MMM + SES: 20 Promise warnings RN/MMM + SES: 20 Promise warnings Aug 17, 2023
@leotm
Copy link
Member Author

leotm commented Dec 5, 2023

no more warnings with SES baked into RN core (instead of app entry file) in

@leotm leotm closed this as completed Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android iOS jsc JavaScriptCore lockdown promise Promise polyfill v8 Chrome V8 warning build/runtime remaining successful
Projects
None yet
Development

No branches or pull requests

1 participant