Skip to content

Conversation

mganandraj
Copy link

@mganandraj mganandraj commented Jan 16, 2020

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

With this change, i'm trying to cleanup a lot of differences that we've managed to accumulate over the last couple of years in JS engine integration. Specifically, we had our own implementation of JSEExecutor interface called V8Executor (Just like CHakraExecutor on Win32/UWP). And we used to switch between V8Executor and JSCExecutor. The switch and the V8Executor remained even though JSCExecutor already went away, which made the switch useless, but most of the code remained which clutterd our build scripts.

With this change, i originally intented to remove the switch and cleanup the build scripts. But, in the process, it felt a lot more efficient to get rid of the V8Executor and switch to JSI engines altogether, as the FB repo (even v0.60.0) has only JSI based executors now. It'd be a backward step to even keep the V8Executor around.

Hence, with this change, we take a big stride forward, by switching to V8 based JSI runtime exclusively. I've already tested RNTester, Modern commenting and LPC already.

This change may a bit aggressive, but i think this will help us a lot in the long time with maintainabiltiy and even performance. Now that we are going to switch to JSI runtimes in Windows very soon, this change will help us to unify the JSI implementation across platforms.

There is a small change in Office code required which i'll send a CR for androidxupgrade override.

Focus areas to test

SDX functionalities.

Microsoft Reviewers: Open in CodeFlow

@pull-bot
Copy link

pull-bot commented Jan 16, 2020

Messages
📖

📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

📖 📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.
📖 📋 Missing Summary - Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.

Generated by 🚫 dangerJS against 0e171e5

Copy link

@BarinderGrewal BarinderGrewal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@mganandraj mganandraj merged commit f5a5da1 into microsoft:master Jan 17, 2020
@mganandraj mganandraj deleted the V8JSI branch January 17, 2020 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants