microsoft/rnx-kit

Lerna 7+ breaks tools-workspaces functionality

poniraq opened this issue · 2 comments

poniraq commented

What happened?

Problem

Lerna 7.0.0 introduced a breaking change that deprecates useWorkspaces. Instead Lerna now relies on a npmClient property to detect workspaces, unless package list is overriden by a packages property.

This in turn breaks the @rnx-kit/metro-config, since it is no longer able to correctly populate watchFolders property for metro config.

Workarounds

  1. duplicate package list in lerna.json this "theoretically" should allow tools-workspaces to proceed as usual.
  2. manually populate watchFolders metro config property.

Solution

My initial impulse was to simply cut everything lerna-related from tools-workspaces, as Lerna seemingly no longer considers itself a "sentinel". This presents 2 problems: a) how do we keep compatibility with Lerna pre-7 versions that still rely on useWorkspaces? b) how do we keep compatibility with Lernas ability to override package list through packages property?

Code in question (packages/tools-workspaces/src/lerna.ts):

// https://lerna.js.org/docs/configuration
export async function findWorkspacePackages(
  configFile: string
): Promise<string[]> {
  const { packages, useWorkspaces } = await readJSON(configFile);
  if (!useWorkspaces) {
    return await findPackages(packages, path.dirname(configFile));
  }

  const root = path.dirname(configFile);
  const sentinels = filterSentinels();
  for (const sentinel of sentinels) {
    const filename = path.join(root, sentinel);
    if (fileExists(filename)) {
      const { findWorkspacePackages } = await getImplementation(filename);
      return await findWorkspacePackages(filename);
    }
  }

  return [];
}

so my way of thinking is that we:

  1. check for packages first -> proceed as usual if found
  2. do not check for useWorkspaces -> try to fallback to other sentinels regardless

like this:

// https://lerna.js.org/docs/configuration
export async function findWorkspacePackages(
  configFile: string
): Promise<string[]> {
  const { packages } = await readJSON(configFile);
  // respect the Lerna `packages` property
  if (packages) {
    return await findPackages(packages, path.dirname(configFile));
  }

  // fallback to alternative sentinels regardless of `useWorkspaces`
  const root = path.dirname(configFile);
  const sentinels = filterSentinels();
  for (const sentinel of sentinels) {
    const filename = path.join(root, sentinel);
    if (fileExists(filename)) {
      const { findWorkspacePackages } = await getImplementation(filename);
      return await findWorkspacePackages(filename);
    }
  }

  return [];
}

The only uncovered case is when user deliberately sets useWorkspaces to false, does not provide lerna with packages but still expects things to somehow magically work. Please tell me if you're aware of such a setup.

Affected Package

@rnx-kit/tools-workspaces

Version

0.1.3

Which platforms are you seeing this issue on?

  • Android
  • iOS
  • macOS
  • Windows

System Information

System:
    OS: Windows 10 10.0.19045
    CPU: (8) x64 Intel(R) Core(TM) i7-9700K CPU @ 3.60GHz        
    Memory: 18.37 GB / 31.93 GB
  Binaries:
    Node: 18.16.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
    npm: 9.5.1 - C:\Program Files\nodejs\npm.CMD
    Watchman: Not Found
  SDKs:
    Android SDK: Not Found
    Windows SDK:
      AllowAllTrustedApps: Enabled
      Versions: 10.0.19041.0
  IDEs:
    Android Studio: AI-222.4459.24.2221.10121639
    Visual Studio: 16.11.33801.447 (Visual Studio Community 2019)
  Languages:
    Java: 11.0.18
  npmPackages:
    @react-native-community/cli: ^10.0.0 => 10.2.5
    react: 18.2.0 => 18.2.0
    react-native: ^0.71.5 => 0.71.11
    react-native-windows: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps to Reproduce

Here's the minimal reproduction example. I'm not familiar with yarn workspaces, so the only limitation here is that it requires pnpm.

https://github.com/poniraq/lerna7-tools-workspaces-bug-example

Code of Conduct

  • I agree to follow this project's Code of Conduct
kelset commented

Thanks for reporting this @poniraq! And double thanks for writing it all in such details, providing a repro AND sending a PR!

Truly the pinnacle of OSS contribution 😍

Overall the report makes sense and the PR seems reasonable, I just want to check back with @tido64 before proceeding; he's currently on 🏝️ so we'll be a bit slow to do a new release. Can you rely on your workaround for a little longer?

poniraq commented

While making a PR i realized that if i use the @rnx-kit/tools-workspaces directly, i can get identical results by manually calling pnpm sentinel implementation. So sure, no problem. Have a nice 🏝️ @tido64 , make the most of it 😄

  • ❤️1