Autolinking is picking up project files not meant to be autolinked

[tido64]

@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

1. npx react-native --version: 5.0.1
2. npx 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 restore

Expected 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".

[tido64]

cc @asklar

[asklar]

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-1

This issue lacks the environment info requested in the bug template. Please edit your issue report to add this information.

If you are using latest version:

1. npx react-native --version:
2. npx react-native run-windows --info:

Otherwise if --info doesn't exist:

1. react-native -v:
2. npm ls rnpm-plugin-windows:
3. npm ls react-native-windows:
4. node -v:
5. npm -v:
6. yarn --version:

[tido64]

I'm not following what your project structure is, can you please clarify?

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:

example
├── node_modules
│   ├── .generated
│   │   └── windows
│   │       └── ReactTestApp.vcxproj
│   └── react-native-test-app
│       └── windows
│           ├── ReactTestApp
│           │   └── ReactTestApp.vcxproj (only a template, not meant to be autolinked)
│           └── ReactTestApp.sln (only a template, not meant to be autolinked)
└── windows
    └── Example.sln

The double-entry error comes from node_modules/.generated/windows/ReactTestApp.vcxproj being manually added via react-native.config.js, and autolinking picking up node_modules/react-native-test-app/windows/ReactTestApp/ReactTestApp.vcxproj because of the recent change to always pick the one project file if there's only one. However, this can be worked around by adding a Dummy.vcxproj, which will force isRnwDependencyProject to 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 config still populates the windows field, which then triggers the newly added exception when you run react-native autolink-windows.

All of this leads me to believe that as long as there is a windows folder, even if it's a JS only library that happened to have Windows specific JS bits, or a Windows test app, autolink-windows will fail or add projects that shouldn't have been autolinked. I guess the question is whether we want autolink-windows to ignore the package and print a warning like on iOS or macOS instead of just failing outright. Or whether react-native config should've populated the windows field 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 config with Dummy.vcxproj in 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 config with Dummy.vcxprojand ReactTestApp.sln renamed to ReactTestApp.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]

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]

the exception would only be triggered if the dependency's name include "react-native" - as we expect native modules to have that prefix.

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:

• metro-react-native-babel-preset
• @react-native-community/hooks
• react-native-dotenv
• react-native-qrcode-svg
• react-native-redash
• react-native-router-flux
• react-native-vector-icons

These packages would throw an exception with this assumption.

[NickGerleman]

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]

@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 DynamicLibrary or something like that.

@NickGerleman this is not on stable branches, only in main, so it should not block anyone.