Update lib to react-native-windows 0.82 new arch#138
Conversation
As of react-native-windows 0.82, support for old arch has been dropped. This commit updates the lib and the example app to new architecture.
birdofpreyru
left a comment
There was a problem hiding this comment.
Thanks @hsjoberg !
Visually, just looking at the PR, it all looks good to me. I had just a few comments, I can correct all the stuff mentioned myself, if you don't feel strong about any of my points, and don't explain me why it should be different.
I'll merge / test the build / release the new lib versions within the next few days.
| # Use this property to enable or disable the Hermes JS engine. | ||
| # If set to false, you will be using JSC instead. | ||
| hermesEnabled=true | ||
| hermesV1Enabled=true |
There was a problem hiding this comment.
Why do we need it? I don't think I had any problems without it, and RN docs for v0.84 tell Hermes V1 is the default with no need for any config changes?
https://reactnative.dev/blog/2026/02/11/react-native-0.84#hermes-v1-as-default
There was a problem hiding this comment.
It is mysterious yes as Hermes V1 is the default now.
However, I ran into gradle dependencies issues without setting this flag explicitly.
FAILURE: Build failed with an exception.
* What went wrong:
Could not determine the dependencies of task ':app:compileDebugJavaWithJavac'.
> Could not resolve all dependencies for configuration ':app:debugCompileClasspath'.
> Could not find com.facebook.react:hermes-android:0.84.0.
Required by:
project ':app'
I guess if it works for you in the example app without then should be fine. Maybe some local machine shenanigans.
I can dig deeper on this one if you want.
There was a problem hiding this comment.
It looks like build problems on Android without this flag were caused by having react-native-windows@0.82 installed alongside react-native@0.84. Gonna remove react-native-windows for now from all package.json files, so that it does not mess anything on other platforms, and to run the example on Windows it will be necessary to install it explicitly.
There was a problem hiding this comment.
I see. Yeah that's reasonable.
Hopefully react-native-windows gets up-to-date soon.
| } | ||
| }, | ||
| 'stopUpload() should stop an upload process [iOS]': async () => { | ||
| if (Platform.OS === 'windows') return Result.notAvailable('windows'); |
There was a problem hiding this comment.
I guess, this is effectively covered by the next line? Also the argument(s) of .notAvailable() is (are) the platforms where the feature is available / test should work; so to get correct error messages, all previous instance of .notAvailable() you added should have different arguments (and probably, it is a good idea for me to do something with that helper function to be less confusing).
There was a problem hiding this comment.
Yes, good point this is silly. No need.
| const Server = require( | ||
| '@dr.pogodin/react-native-static-server', | ||
| ).default as typeof import('@dr.pogodin/react-native-static-server').default; |
There was a problem hiding this comment.
Hmm... wouldn't it work with just?
const Server = await import('@dr.pogodin/react-native-static-server');There was a problem hiding this comment.
It is enough yes, however adding as typeof import('@dr.pogodin/react-native-static-server').default; gave type info (any otherwise for me).
| "react": "19.2.3", | ||
| "react-native": "0.84.0", | ||
| "react-native-builder-bob": "^0.40.18", | ||
| "react-native-windows": "0.82.0", |
There was a problem hiding this comment.
We prefer to not include react-native-windows as the dependency into the main package.json... if it is really needed on that level, it should be a dev dependency. Many RN devs only do Android / iOS, and don't want to automatically install with the library rnw and its transient dependencies, unnecessary for them. Also, because rnw lags behind the main rn, with some mismatching versions pairs in past I so some weird dependency conflicts, and somewhat confusing build failures for other platforms.
There was a problem hiding this comment.
This is in devDependencies actually, not dependencies.
I think we need this one there for running windows codegen.
|
Thank you for the review @birdofpreyru.
Sure all good by me. |
Hi, this PR migrates Windows to rnw new arch.
Old arch support was removed upstream in rnw 0.82, so this change only targets new arch.
As we know, rnw drifted behind core react-native, but I've kept
react-nativeitself at 0.84.What changed
cpp-lib/cpp-appscaffoldingReactNativeModule.cpp):IInitializeWithWindowMainBundlePathsafe for unpackaged appsbuild/targetoutputs are properly blacklisted on Windows@dr.pogodin/react-native-static-serveron Windows only in the example app for now, as that lib also needs migrationhermesV1Enabled=truein the Android example so Hermes resolves correctly with react-native 0.84pickFile()button to blue because it was otherwise invisible on rnwVerification
yarn example windows)Note
First time building fails with
Just build again and should work. More info: microsoft/react-native-windows#15747.
Closes #123, #116