Merge pull request #32562 from Believe-SA/package-workspaces

chore: replace legacy package.json workspaces.packages by workspaces

.changeset/slick-dogs-pay.md

@@ -0,0 +1,7 @@
+---
+'@backstage/create-app': patch
+'@backstage/cli-node': patch
+'@backstage/cli': patch
+---
+
+Properly support `package.json` `workspaces` field

.changeset/slow-items-pull.md

@@ -0,0 +1,18 @@
+---
+'@backstage/create-app': patch
+---
+
+Replace deprecated `workspaces.packages` with `workspaces` in `package.json`
+
+This change is **not** required, but you can edit your main `package.json`, to fit the more modern & more common pattern:
+
+```diff
+-  "workspaces": {
+-    "packages": [
+   "workspaces": [
+     "packages/*",
+     "plugins/*"
+-     ]
+-   },
+  ],
+```

docs/contribute/project-structure.md

@@ -55,12 +55,10 @@ defined in
 [`package.json`](https://github.com/backstage/backstage/blob/master/package.json):
 
 ```json
- "workspaces": {
-    "packages": [
-      "packages/*",
-      "plugins/*"
-    ]
-  },
+  "workspaces": [
+    "packages/*",
+    "plugins/*"
+  ],
 ```
 
 Let's look at them individually.

package.json

@@ -16,12 +16,10 @@
     "type": "git",
     "url": "https://github.com/backstage/backstage"
   },
-  "workspaces": {
-    "packages": [
-      "packages/*",
-      "plugins/*"
-    ]
-  },
+  "workspaces": [
+    "packages/*",
+    "plugins/*"
+  ],
   "scripts": {
     "build-storybook": "storybook build --output-dir dist-storybook",
     "build-storybook:chromatic": "STORYBOOK_STORY_SET=chromatic storybook build --stats-json --output-dir dist-storybook",

packages/cli-common/src/paths.test.ts

@@ -98,7 +98,9 @@ describe('paths', () => {
   });
 
   it('findPaths should find workspace root with object', () => {
-    jest.spyOn(JSON, 'parse').mockReturnValue({ workspaces: { packages: [] } });
+    jest
+      .spyOn(JSON, 'parse')
+      .mockReturnValue({ workspaces: { packages: ['packages/*'] } });
     jest.spyOn(process, 'cwd').mockReturnValue(__dirname);
 
     const paths = findPaths(__dirname);
@@ -110,7 +112,7 @@ describe('paths', () => {
   });
 
   it('findPaths should find workspace root with array', () => {
-    jest.spyOn(JSON, 'parse').mockReturnValue({ workspaces: [] });
+    jest.spyOn(JSON, 'parse').mockReturnValue({ workspaces: ['packages/*'] });
     jest.spyOn(process, 'cwd').mockReturnValue(__dirname);
 
     const paths = findPaths(__dirname);

packages/cli-node/src/monorepo/isMonoRepo.ts

@@ -18,7 +18,10 @@ import { targetPaths } from '@backstage/cli-common';
 import fs from 'fs-extra';
 
 /**
- * Returns try if the current project is a monorepo.
+ * Returns true if the current project is a monorepo.
+ *
+ * Uses a simple presence check on the `workspaces` field. Empty or invalid
+ * workspace config is treated as a monorepo; we do not validate patterns.
  *
  * @public
  */
@@ -26,7 +29,7 @@ export async function isMonoRepo(): Promise<boolean> {
   const rootPackageJsonPath = targetPaths.resolveRoot('package.json');
   try {
     const pkg = await fs.readJson(rootPackageJsonPath);
-    return Boolean(pkg?.workspaces?.packages);
+    return Boolean(pkg?.workspaces);
   } catch (error) {
     return false;
   }

packages/cli-node/src/pacman/PackageManager.test.ts

@@ -52,6 +52,16 @@ describe('PackageManager', () => {
         await detectPackageManager();
         expect(mockYarnCreate).toHaveBeenCalled();
       });
+      it('should detect via root package.json workspaces (array form)', async () => {
+        mockDir.setContent({
+          'package.json': JSON.stringify({
+            name: 'foo',
+            workspaces: ['packages/*'],
+          }),
+        });
+        await detectPackageManager();
+        expect(mockYarnCreate).toHaveBeenCalled();
+      });
 
       it('should detect via root package.json packageManager', async () => {
         mockDir.setContent({

packages/cli-node/src/pacman/PackageManager.ts

@@ -49,12 +49,6 @@ export interface PackageManager {
   /** The file name of the lockfile used by the package manager. */
   lockfileName(): string;
 
-  /**
-   * If this repo is a monorepo, returns the patterns specified by the package
-   * manager's monorepo configuration. Does not attempt to resolve any globs.
-   */
-  getMonorepoPackages(): Promise<string[]>;
-
   /** Uses the package manager to run a command in the repo. */
   run(args: string[], options?: RunOptions): Promise<void>;
 

packages/cli-node/src/pacman/yarn/Yarn.test.ts

@@ -1,56 +0,0 @@
-/*
- * Copyright 2024 The Backstage Authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import { createMockDirectory } from '@backstage/backend-test-utils';
-import { overrideTargetPaths } from '@backstage/cli-common/testUtils';
-import { Yarn } from './Yarn';
-
-const mockDir = createMockDirectory();
-overrideTargetPaths(mockDir.path);
-
-const yarnClassic = new Yarn({ version: '1.0.0', codename: 'classic' });
-const yarnBerry = new Yarn({ version: '3.0.0', codename: 'berry' });
-const allYarnVersions = [yarnClassic, yarnBerry];
-
-describe('Yarn', () => {
-  describe.each(allYarnVersions)('%s.getMonorepoPackages', yarn => {
-    it('should detect a monorepo', async () => {
-      mockDir.setContent({
-        'package.json': JSON.stringify({
-          name: 'foo',
-          workspaces: {
-            packages: ['packages/*'],
-          },
-        }),
-      });
-      await expect(yarn.getMonorepoPackages()).resolves.toEqual(['packages/*']);
-    });
-
-    it('should detect a non-monorepo', async () => {
-      mockDir.setContent({
-        'package.json': JSON.stringify({
-          name: 'foo',
-        }),
-      });
-      await expect(yarn.getMonorepoPackages()).resolves.toEqual([]);
-    });
-
-    it('should return false if package.json is missing', async () => {
-      mockDir.setContent({});
-      await expect(yarn.getMonorepoPackages()).resolves.toEqual([]);
-    });
-  });
-});

packages/cli-node/src/pacman/yarn/Yarn.ts

@@ -22,8 +22,6 @@ import {
 import { PackageInfo, PackageManager } from '../PackageManager';
 import { Lockfile } from '../Lockfile';
 import { YarnVersion } from './types';
-import fs from 'fs-extra';
-import { targetPaths } from '@backstage/cli-common';
 import { run, runOutput, RunOptions } from '@backstage/cli-common';
 
 export class Yarn implements PackageManager {
@@ -46,16 +44,6 @@ export class Yarn implements PackageManager {
     return 'yarn.lock';
   }
 
-  async getMonorepoPackages() {
-    const rootPackageJsonPath = targetPaths.resolveRoot('package.json');
-    try {
-      const pkg = await fs.readJson(rootPackageJsonPath);
-      return pkg?.workspaces?.packages || [];
-    } catch (error) {
-      return [];
-    }
-  }
-
   async pack(out: string, packageDir: string) {
     const outArg =
       this.yarnVersion.codename === 'classic' ? '--filename' : '--out';

packages/cli/config/jest.js

@@ -336,8 +336,8 @@ async function getRootConfig() {
     rejectFrontendNetworkRequests,
   };
 
-  const workspacePatterns =
-    rootPkgJson.workspaces && rootPkgJson.workspaces.packages;
+  const ws = rootPkgJson.workspaces;
+  const workspacePatterns = Array.isArray(ws) ? ws : ws?.packages;
 
   // Check if we're running within a specific monorepo package. In that case just get the single project config.
   if (!workspacePatterns || paths.targetRoot !== paths.targetDir) {

packages/cli/src/modules/migrate/commands/versions/bump.test.ts

@@ -150,9 +150,7 @@ describe('bump', () => {
     mockDir.setContent({
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -245,9 +243,7 @@ describe('bump', () => {
     mockDir.setContent({
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -343,9 +339,7 @@ describe('bump', () => {
     mockDir.setContent({
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -449,9 +443,7 @@ describe('bump', () => {
       '.yarnrc.yml': yarnRcMock,
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -562,9 +554,7 @@ describe('bump', () => {
     mockDir.setContent({
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -634,9 +624,7 @@ describe('bump', () => {
     mockDir.setContent({
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -740,9 +728,7 @@ describe('bump', () => {
     mockDir.setContent({
       'yarn.lock': customLockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -855,9 +841,7 @@ describe('bump', () => {
     mockDir.setContent({
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -1099,9 +1083,7 @@ describe('environment variables', () => {
     mockDir.setContent({
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -1192,9 +1174,7 @@ describe('environment variables', () => {
       'custom-manifest.json': JSON.stringify(customManifest),
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -1262,9 +1242,7 @@ describe('environment variables', () => {
       '.yarnrc.yml': yarnRcMock,
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -1337,9 +1315,7 @@ describe('environment variables', () => {
     mockDir.setContent({
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {
@@ -1362,9 +1338,7 @@ describe('environment variables', () => {
     mockDir.setContent({
       'yarn.lock': lockfileMock,
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       packages: {
         a: {

packages/cli/src/modules/migrate/commands/versions/migrate.test.ts

@@ -73,9 +73,7 @@ describe('versions:migrate', () => {
   it('should bump to the moved version when the package is moved', async () => {
     mockDir.setContent({
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       node_modules: {
         '@backstage': {
@@ -177,9 +175,7 @@ describe('versions:migrate', () => {
   it('should replace the occurrences of the moved package in files inside the correct package', async () => {
     mockDir.setContent({
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       node_modules: {
         '@backstage': {
@@ -264,9 +260,7 @@ describe('versions:migrate', () => {
   it('should replace occurrences of changed packages, and is careful', async () => {
     mockDir.setContent({
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['packages/*'],
-        },
+        workspaces: ['packages/*'],
       }),
       node_modules: {
         '@backstage': {

packages/cli/src/modules/migrate/lib/versioning/packages.test.ts

@@ -104,9 +104,7 @@ describe('mapDependencies', () => {
   it('should read dependencies', async () => {
     mockDir.setContent({
       'package.json': JSON.stringify({
-        workspaces: {
-          packages: ['pkgs/*'],
-        },
+        workspaces: ['pkgs/*'],
       }),
       pkgs: {
         a: {

packages/create-app/templates/default-app/package.json.hbs

@@ -22,12 +22,10 @@
     "prettier:check": "prettier --check .",
     "new": "backstage-cli new"
   },
-  "workspaces": {
-    "packages": [
-      "packages/*",
-      "plugins/*"
-    ]
-  },
+  "workspaces": [
+    "packages/*",
+    "plugins/*"
+  ],
   "devDependencies": {
     "@backstage/cli": "^{{version '@backstage/cli'}}",
     "@backstage/e2e-test-utils": "^{{version '@backstage/e2e-test-utils'}}",

packages/create-app/templates/next-app/package.json.hbs

@@ -44,12 +44,10 @@
       }
     }
   },
-  "workspaces": {
-    "packages": [
-      "packages/*",
-      "plugins/*"
-    ]
-  },
+  "workspaces": [
+    "packages/*",
+    "plugins/*"
+  ],
   "devDependencies": {
     "@backstage/cli": "^{{version '@backstage/cli'}}",
     "@backstage/e2e-test-utils": "^{{version '@backstage/e2e-test-utils'}}",