Repository navigation
Autolinking is picking up project files not meant to be autolinked #8072
Description
@react-native-windows/cli is adding a project file that wasn't meant to be autolinked. This was probably introduced in 11a322b.
Correct me if I'm wrong, but it seems like we are assuming that if react-native config populates the windows field, it must be a react-native-windows compatible project. This will break JS-only libraries that have a Windows test app somewhere in the package. As far as I can see, there are two solutions to this, either rename the windows folder to something else, or disable autolinking of the package altogether, i.e.:
// react-native.config.js
module.exports = {
dependency: {
platforms: {
windows: null,
},
},
};Environment
npx react-native --version: 5.0.1npx react-native info:System: OS: Windows 10 10.0.19041 CPU: (4) x64 Intel(R) Core(TM) i5-4690K CPU @ 3.50GHz Memory: 7.33 GB / 15.45 GB Binaries: Node: 14.16.1 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.5 - C:\Program Files (x86)\Yarn\bin\yarn.CMD npm: 6.14.12 - C:\Program Files\nodejs\npm.CMD npmPackages: react: 17.0.2 => 17.0.2 react-native: ^0.64.2 => 0.64.2 react-native-windows: canary => 0.0.0-canary.327 Installed UWP SDKs: 10.0.18362.0 10.0.19041.0
- Target Device(s): Desktop
- Visual Studio Version: 2019
Steps To Reproduce
git clone https://github.com/microsoft/react-native-test-app.git
cd react-native-test-app
yarn set-react-version canary-windows
cd example
yarn
yarn install-windows-test-app --use-nuget
cd windows
nuget restoreExpected Results
nuget restore to succeed. Instead, I'm getting an error The solution file has two projects named "ReactTestApp" because a .vcxproj was added twice. I can work around this error by adding a Dummy.vcxproj to force isRnwDependencyProject to be called, but then I get an exception Found a Windows solution for ReactTestApp but no React Native for Windows native module projects.
Snack, code example, screenshot, or link to a repository:
See "Steps To Reproduce".
Activity
tido64 commentedon Jun 20, 2021
cc Alexander Sklar (@asklar)
asklar commentedon Jun 20, 2021
I'm not following what your project structure is, can you please clarify?
Projects can set the directDependency field to false to mark projects not to be autolinked, see
https://microsoft.github.io/react-native-windows/docs/config#project-object-fields-1tido64 commentedon Jun 20, 2021
Sorry for not providing all the details as I was still trying to understand what triggered all the errors. The project structure looks like this:
The double-entry error comes from
node_modules/.generated/windows/ReactTestApp.vcxprojbeing manually added viareact-native.config.js, and autolinking picking upnode_modules/react-native-test-app/windows/ReactTestApp/ReactTestApp.vcxprojbecause of the recent change to always pick the one project file if there's only one. However, this can be worked around by adding aDummy.vcxproj, which will forceisRnwDependencyProjectto be called since there are more than one project file. With the dummy file in place, neither projects are autolinked as expected. However,react-native configstill populates thewindowsfield, which then triggers the newly added exception when you runreact-native autolink-windows.All of this leads me to believe that as long as there is a
windowsfolder, even if it's a JS only library that happened to have Windows specific JS bits, or a Windows test app,autolink-windowswill fail or add projects that shouldn't have been autolinked. I guess the question is whether we wantautolink-windowsto ignore the package and print a warning like on iOS or macOS instead of just failing outright. Or whetherreact-native configshould've populated thewindowsfield in the first place if there was nothing there.For reference, here's the output of
react-native config:{ "root": "D:\\Source\\react-native-test-app\\example", "reactNativePath": "D:\\Source\\react-native-test-app\\example\\node_modules\\react-native", "dependencies": { "react-native-test-app": { "root": "D:\\Source\\react-native-test-app\\example\\node_modules\\react-native-test-app", "name": "react-native-test-app", "platforms": { "ios": { ... }, "android": null, "windows": { "folder": "D:\\Source\\react-native-test-app\\example\\node_modules\\react-native-test-app", "projects": [ { "projectFile": "ReactTestApp\\ReactTestApp.vcxproj", "projectName": "ReactTestApp", "projectLang": "cpp", "projectGuid": "{b44cead7-fbff-4a17-95ea-ff5434bbd79d}", "directDependency": true, "cppHeaders": [ "winrt/ReactTestApp.h" ], "cppPackageProviders": [ "ReactTestApp::ReactPackageProvider" ], "csNamespaces": [ "ReactTestApp" ], "csPackageProviders": [ "ReactTestApp.ReactPackageProvider" ] } ], "solutionFile": "ReactTestApp.sln", "nugetPackages": [], "sourceDir": "windows" }, "assets": [], "hooks": {}, "params": [] } }, ... }Output of
react-native configwithDummy.vcxprojin place:{ ... "windows": { "folder": "D:\\Source\\react-native-test-app\\example\\node_modules\\react-native-test-app", "projects": [], "solutionFile": "ReactTestApp.sln", "nugetPackages": [], "sourceDir": "windows" }, "assets": [], "hooks": {}, "params": [] } }, ... }Output of
react-native configwithDummy.vcxprojandReactTestApp.slnrenamed toReactTestApp.sln.template:{ ... "windows": { "folder": "D:\\Source\\react-native-test-app\\example\\node_modules\\react-native-test-app", "projects": [], "solutionFile": null, "nugetPackages": [], "sourceDir": "windows" }, "assets": [], "hooks": {}, "params": [] } }, ... }asklar commentedon Jun 20, 2021
the exception would only be triggered if the dependency's name include "react-native" - as we expect native modules to have that prefix. We detect something might be a dependency if it has an SLN. If it has an SLN, then if it has exactly one vcxproj or csproj, we'll autolink that. If it has more than one, we will rely on the config if it exists, or fall back to checking whether the projects import the native module properties.
It sounds like this folder structure isn't really something we support, can you clarify what scenario is this serving? I'd like to make sure your project isn't deviating from guardrails/assumptions we are putting in place in RNW.
tido64 commentedon Jun 21, 2021
I don't think you can safely assume this. Looking at npm, I found a few JS-only packages with "react-native" in the name:
These packages would throw an exception with this assumption.
NickGerleman commentedon Jun 22, 2021
I think us quickly hitting the issue in an MSFT repository is good signal that it could affect others.
Can we revise this logic to better handle non-NM packages? I would consider temporarily pulling this out of the stable branches if partners are blocked on building with it.
asklar commentedon Jun 22, 2021
Tommy Nguyen (@tido64) those JS only projects shouldn't hit the issue since they don't have a windows SLN file :)
I think the problem may be that your package is adding a test app with a solution but the test app isn't detected as "the app" if that makes sense, and instead it is detected as a module. We could possibly tighten the heuristic to only consider vcxprojs if they are of type
DynamicLibraryor something like that.Nick Gerleman (@NickGerleman) this is not on stable branches, only in main, so it should not block anyone.
NickGerleman commentedon Jun 22, 2021
You're right. Missed this was a canary build. (Thank you Tommy Nguyen (@tido64) for testing them).
The idea of tightening the heuristic makes sense to me.
NickGerleman commentedon Jun 24, 2021
Tommy Nguyen (@tido64) is doing some cross-msft repo work on CXE. Going to mark this one so we give it some discussion as part of incoming issues, since it affects internal.
I will forward an instance of the meeting in case you would like to join.
tido64 commentedon Jun 24, 2021
Nick Gerleman (@NickGerleman) Thanks.
Alexander Sklar (@asklar) That sounds reasonable to me. I whipped up a PR. Let me know if it looks okay.