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

Defork JSIDynamic.cpp and JSIDynamic.h #12581

Closed
TatianaKapos opened this issue Jan 10, 2024 · 3 comments
Closed

Defork JSIDynamic.cpp and JSIDynamic.h #12581

TatianaKapos opened this issue Jan 10, 2024 · 3 comments

Comments

@TatianaKapos
Copy link
Contributor

TatianaKapos commented Jan 10, 2024

Problem Description

Recent integration updated brough in this PR which updated JSIDynamic.cpp and JSIDynamic.h. For some reason, some of our build configurations don't update those files and still pulls in the old versions. Currently fixed this by forcing them to use the new files by adding an override but we should figure out a way to defork these files.

List of failing tests (Looks like mostly Fabric on Hermes)
image

My suspicion is hermes-windows holds onto the old versions of these files https://github.com/microsoft/hermes-windows/blob/main/API/jsi/jsi/JSIDynamic.cpp and needs to be updated.

Steps To Reproduce

  1. remove override for JSIDYnamic
  2. run pipeline

Expected Results

should run fine

CLI version

npx react-native -v

Environment

npx react-native info

Target Platform Version

10.0.19041

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 Jan 10, 2024
@TatianaKapos TatianaKapos changed the title Fix Fabric Defork JSIDynamic.cpp and JSIDynamic.h Jan 10, 2024
@TatianaKapos
Copy link
Contributor Author

@chrisglein
Copy link
Member

@vmoroz It sounds like Hermes and RNW are fighting over overrides to these files. Is this an issue where the versions need to be in sync and aren't? What's the right way to proceed so that they line up.

@chrisglein chrisglein added Area: JSI Integration Follow-up and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Jan 11, 2024
@chrisglein chrisglein added this to the Backlog milestone Jan 11, 2024
@TatianaKapos
Copy link
Contributor Author

closing! Fixed in #12571

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants