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

Unfork JSRuntimeFactory Files #13172

Open
TatianaKapos opened this issue May 1, 2024 · 1 comment
Open

Unfork JSRuntimeFactory Files #13172

TatianaKapos opened this issue May 1, 2024 · 1 comment

Comments

@TatianaKapos
Copy link
Contributor

TatianaKapos commented May 1, 2024

Problem Description

Recent integration brought in this PR which added the method getRuntimeTargetDelegate to JSRuntime and changes how we pass the RuntimeTarget. HermesRuntime does not implement this method but because it's a virtual method, Hermes needs too if we want to unfork these files.

Current Forked Files

  • JSExecutor.cpp
  • JSExector.h
  • NativeToJsBridge.cpp
  • JSRuntimeFactory.cpp
  • JSRuntimeFactory.h
  • ReactInstance.cpp

Currently these files are set to an older version in the integration.

Steps To Reproduce

  1. Remove override
  2. Will get this error message Error LNK2001 unresolved external symbol "public: virtual class facebook::react::jsinspector_modern::RuntimeTargetDelegate & __cdecl facebook::react::JSRuntime::getRuntimeTargetDelegate(void)" (?getRuntimeTargetDelegate@JSRuntime@react@facebook@@UEAAAEAVRuntimeTargetDelegate@jsinspector_modern@23@XZ) Microsoft.ReactNative D:\react-native-windows\vnext\Microsoft.ReactNative\HermesRuntimeHolder.obj

Expected Results

Should not get an error message

CLI version

npx react-native -v

Environment

npx react-native info

Target Platform Version

10.0.22321

Target Device(s)

Desktop

Visual Studio Version

Visual Studio 2022

Build Configuration

Debug

Snack, code example, screenshot, or link to a repository

No response

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label May 1, 2024
@chrisglein chrisglein added this to the Next milestone May 1, 2024
@chrisglein
Copy link
Member

Based on getRuntimeTargetDelegate and jsinspector_modern this may be related to the debugging changes.
Overrides have integration blocked to move. This is from post 0.74 main changes.
Given the amount of traffic we'll be having to get hermes direct debugging working in 0.74, there will certainly be conflicts with these forks as we move forward.

@chrisglein chrisglein added enhancement and removed bug Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants