fix: optimize workspace package resolution in dependency tree

[beyondkmp]

fix #8956

Root Cause
The module in the workspace uses scoped naming convention like @packages/app, while the root directory is ended with packages/app. This mismatch results in returning empty and consequently leads to no dependencies being found.

electron-builder/packages/app-builder-lib/src/node-module-collector/nodeModulesCollector.ts

Lines 150 to 152 in 7ba4fea

	for (const [key, value] of Object.entries(tree.dependencies)) {
	if (this.rootDir.endsWith(path.normalize(key))) {
	return value

How to fix
Read the name from package.json in the root directory and compare it with the keys in dependencies, this way it can match even with arbitrary scoped names (@xxx/yyyy).

[changeset-bot]

🦋 Changeset detected

Latest commit: 1998b64

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages

Name	Type
app-builder-lib	Patch
dmg-builder	Patch
electron-builder-squirrel-windows	Patch
electron-builder	Patch
electron-forge-maker-appimage	Patch
electron-forge-maker-nsis-web	Patch
electron-forge-maker-nsis	Patch
electron-forge-maker-snap	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[mmaietta]

Do we need to add a new fixture or at minimum, not commit the node_modules to the repo? Is it possible to use an existing fixture? Seems like we could simply use

await modifyPackageJson(projectDir, data => {
  data.dependencies = {
     "is-odd": "^3.0.1"
  }
}),

With this to save the lockfile

isInstallDepsBefore: true

[beyondkmp]

 test("yarn workspace for scope name", ({ expect }) =>
  assertPack(
    expect,
    "test-app-yarn-several-workspace",
    {
      targets: linuxDirTarget,
      projectDir: "packages/test-app",
    },
    {
      isInstallDepsBefore: true,
      projectDirCreated: projectDir => {
          return Promise.all([
            modifyPackageJson(projectDir, data => {
              data.name = "@scope/xxx-app"
              data.dependencies = {
                "is-odd": "3.0.1"
              }
            }),
          ])
        },

      packed: context => verifyAsarFileTree(expect, context.getResources(Platform.LINUX)),
    }
  ))

The modification of package.json is incorrectly targeting the project's root package.json instead of the one in packages/test-app directory. This appears to be a bug.

[beyondkmp]

@mmaietta I concatenated it manually. The unit test issues have been resolved. Please help review it when you have time.

test("yarn workspace for scope name", ({ expect }) =>
  assertPack(
    expect,
    "test-app-yarn-several-workspace",
    {
      targets: linuxDirTarget,
      projectDir: "packages/test-app",
    },
    {
      isInstallDepsBefore: true,
      projectDirCreated: projectDir => {
            let subAppDir = path.join(projectDir , "packages","test-app")
            return modifyPackageJson(subAppDir, data => {
              data.name = "@scope/xxx-app"
              data.dependencies = {
                "is-odd": "3.0.1"
              }
            })
        },
      packed: context => verifyAsarFileTree(expect, context.getResources(Platform.LINUX)),
    }
  ))

[mmaietta]

The modification of package.json is incorrectly targeting the project's root package.json instead of the one in packages/test-app directory. This appears to be a bug.

Not a bug, it's designed to work with both one-package.json and two-package.json setups but with the projectDir expected to be under app and not test-app (so part of the test-app) fixture. To flip to two-package.json "mode", it's simply setting the optional isApp boolean

electron-builder/test/src/helpers/packTester.ts

Lines 546 to 547 in 7ba4fea

	export async function modifyPackageJson(projectDir: string, task: (data: any) => void, isApp = false): Promise<any> {
	const file = isApp ? path.join(projectDir, "app", "package.json") : path.join(projectDir, "package.json")

Much cleaner diff now though, thank you for making the modifications!

[Lemonexe]

Thank you, @beyondkmp 🥇
With app-builder-lib built from master, I confirmed it to be working both in the minimal repro, as well as in our slightly more complex production app ✔️ 👍