fix: allow dependencies without a package.json (#3782)

close #1866

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/fair-boats-tell.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/lifecycle": patch
+"@pnpm/prepare-package": patch
+---
+
+Packages that have no `package.json` files should be skipped.

fixtures/package.yaml

@@ -1,5 +1,5 @@
 scripts:
   prepareFixtures: >
-    pnpm install -rf --frozen-lockfile --no-shared-workspace-lockfile --no-link-workspace-packages &&
-    pnpm install -rf -C fixtureWithLinks --frozen-lockfile --link-workspace-packages --no-shared-workspace-lockfile &&
-    cd ./fixture-with-external-shrinkwrap/pkg && pnpm install -f --frozen-lockfile
+    node ../packages/pnpm/dist/pnpm.cjs install -rf --frozen-lockfile --no-shared-workspace-lockfile --no-link-workspace-packages &&
+    node ../packages/pnpm/dist/pnpm.cjs install -rf -C fixtureWithLinks --frozen-lockfile --link-workspace-packages --no-shared-workspace-lockfile &&
+    cd ./fixture-with-external-shrinkwrap/pkg && node ../../../packages/pnpm/dist/pnpm.cjs install -f --frozen-lockfile

fixtures/with-non-package-dep/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "with-non-package-dep",
+  "version": "1.0.0",
+  "dependencies": {
+    "camelcase": "denolib/camelcase#aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b"
+  }
+}

fixtures/with-non-package-dep/pnpm-lock.yaml

@@ -0,0 +1,15 @@
+lockfileVersion: 5.3
+
+specifiers:
+  camelcase: denolib/camelcase#aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b
+
+dependencies:
+  camelcase: github.com/denolib/camelcase/aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b
+
+packages:
+
+  github.com/denolib/camelcase/aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b:
+    resolution: {tarball: https://codeload.github.com/denolib/camelcase/tar.gz/aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b}
+    name: camelcase
+    version: denolib/camelcase#aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b
+    dev: false

package.json

@@ -4,12 +4,13 @@
     "bump": "changeset version && pnpm update-manifests",
     "changeset": "changeset",
     "preinstall": "npx only-allow pnpm",
-    "prepare": "pnpm --dir=verdaccio install && pnpm --dir=fixtures run prepareFixtures && husky install",
+    "prepare": "pnpm --dir=verdaccio install && husky install",
+    "pretest": "pnpm run compile-only && pnpm --dir=fixtures run prepareFixtures",
     "lint": "pnpm lint:meta && syncpack list-mismatches && eslint --config=eslint.json **/src/**/*.ts **/test/**/*.ts",
-    "test-main": "pnpm compile-only && pnpm lint -- --quiet && run-p -r verdaccio test-pkgs-main",
+    "test-main": "pnpm pretest && pnpm lint -- --quiet && run-p -r verdaccio test-pkgs-main",
     "remove-temp-dir": "shx rm -rf ../pnpm_tmp",
     "test-pkgs-main": "pnpm remove-temp-dir && cross-env PNPM_REGISTRY_MOCK_UPLINK=http://localhost:7348 pnpm --filter=./packages/** --filter=./privatePackages/** run --no-sort --workspace-concurrency=2 _test",
-    "test-branch": "pnpm compile-only && pnpm lint -- --quiet && git remote set-branches --add origin main && git fetch && run-p -r verdaccio test-pkgs-branch",
+    "test-branch": "pnpm pretest && pnpm lint -- --quiet && git remote set-branches --add origin main && git fetch && run-p -r verdaccio test-pkgs-branch",
     "test-pkgs-branch": "pnpm remove-temp-dir && cross-env PNPM_REGISTRY_MOCK_UPLINK=http://localhost:7348 pnpm --filter=...[origin/main] run --no-sort _test",
     "verdaccio": "verdaccio --config=./verdaccio.yaml --listen=7348",
     "compile-only": "pnpm --workspace-concurrency=1 --filter=pnpm --filter=@pnpm/make-dedicated-lockfile --filter=@pnpm/mount-modules run compile",

packages/dependencies-hierarchy/test/index.ts

@@ -8,6 +8,7 @@ const generalFixture = path.join(fixtures, 'general')
 const withPeerFixture = path.join(fixtures, 'with-peer')
 const circularFixture = path.join(fixtures, 'circular')
 const withFileDepFixture = path.join(fixtures, 'with-file-dep')
+const withNonPackageDepFixture = path.join(fixtures, 'with-non-package-dep')
 const withLinksOnlyFixture = path.join(fixtures, 'fixtureWithLinks/with-links-only')
 const withUnsavedDepsFixture = path.join(fixtures, 'with-unsaved-deps')
 const fixtureMonorepo = path.join(__dirname, '..', 'fixtureMonorepo')
@@ -482,3 +483,39 @@ test('peer dependencies', async () => {
   expect(hierarchy[withPeerFixture].dependencies![1].dependencies![0].name).toEqual('ajv')
   expect(hierarchy[withPeerFixture].dependencies![1].dependencies![0].isPeer).toEqual(true)
 })
+
+// Test case for https://github.com/pnpm/pnpm/issues/1866
+test('dependency without a package.json', async () => {
+  const org = 'denolib'
+  const pkg = 'camelcase'
+  const commit = 'aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b'
+  const tree = await dh([withNonPackageDepFixture], { depth: 0, lockfileDir: withNonPackageDepFixture })
+  expect(tree).toStrictEqual({
+    [withNonPackageDepFixture]: {
+      dependencies: [
+        {
+          alias: 'camelcase',
+          dev: false,
+          isMissing: false,
+          isPeer: false,
+          isSkipped: false,
+          name: 'camelcase',
+          path: path.join(withNonPackageDepFixture, 'node_modules', '.pnpm', `github.com+${org}+${pkg}@${commit}`),
+          resolved: `https://codeload.github.com/${org}/${pkg}/tar.gz/${commit}`,
+          version: `${org}/${pkg}#${commit}`,
+        },
+      ],
+      devDependencies: [],
+      optionalDependencies: [],
+    },
+  })
+  // verify dependency without a package.json
+  expect(tree[withNonPackageDepFixture].dependencies).toBeDefined()
+  expect(Array.isArray(tree[withNonPackageDepFixture].dependencies)).toBeTruthy()
+  expect(tree[withNonPackageDepFixture].dependencies!.length).toBeGreaterThan(0)
+  expect(tree[withNonPackageDepFixture].dependencies![0]).toBeDefined()
+  // verify that dependency without a package.json has no further dependencies
+  expect(tree[withNonPackageDepFixture].dependencies![0]['dependencies']).toBeUndefined()
+  expect(tree[withNonPackageDepFixture].dependencies![0]['devDependencies']).toBeUndefined()
+  expect(tree[withNonPackageDepFixture].dependencies![0]['optionalDependencies']).toBeUndefined()
+})

packages/git-fetcher/test/index.ts

@@ -44,3 +44,23 @@ test('fetch a package from Git that has a prepare script', async () => {
   )
   expect(filesIndex[`dist${path.sep}index.js`]).toBeTruthy()
 })
+
+// Test case for https://github.com/pnpm/pnpm/issues/1866
+test('fetch a package without a package.json', async () => {
+  const cafsDir = tempy.directory()
+  const fetch = createFetcher().git
+  const manifest = pDefer<DependencyManifest>()
+  const { filesIndex } = await fetch(
+    createCafsStore(cafsDir),
+    {
+      // a small Deno library with a 'denolib.json' instead of a 'package.json'
+      commit: 'aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b',
+      repo: 'https://github.com/denolib/camelcase.git',
+      type: 'git',
+    },
+    {
+      manifest,
+    }
+  )
+  expect(filesIndex['denolib.json']).toBeTruthy()
+})

packages/lifecycle/src/index.ts

@@ -1,5 +1,5 @@
 import path from 'path'
-import { fromDir as readPackageJsonFromDir } from '@pnpm/read-package-json'
+import { safeReadPackageFromDir } from '@pnpm/read-package-json'
 import exists from 'path-exists'
 import runLifecycleHook, { RunLifecycleHookOptions } from './runLifecycleHook'
 import runLifecycleHooksConcurrently, { RunLifecycleHooksConcurrentlyOptions } from './runLifecycleHooksConcurrently'
@@ -20,7 +20,8 @@ export {
 export async function runPostinstallHooks (
   opts: RunLifecycleHookOptions
 ): Promise<boolean> {
-  const pkg = await readPackageJsonFromDir(opts.pkgRoot)
+  const pkg = await safeReadPackageFromDir(opts.pkgRoot)
+  if (pkg == null) return false
   if (pkg.scripts == null) {
     pkg.scripts = {}
   }

packages/package-requester/test/index.ts

@@ -813,3 +813,39 @@ test('do not fetch an optional package that is not installable', async () => {
   expect(pkgResponse.files).toBeFalsy()
   expect(pkgResponse.finishing).toBeFalsy()
 })
+
+// Test case for https://github.com/pnpm/pnpm/issues/1866
+test('fetch a git package without a package.json', async () => {
+  // a small Deno library with a 'denolib.json' instead of a 'package.json'
+  const repo = 'denolib/camelcase'
+  const commit = 'aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b'
+
+  nock.cleanAll()
+  const storeDir = tempy.directory()
+  const cafs = createCafsStore(storeDir)
+  const requestPackage = createPackageRequester({
+    resolve,
+    fetchers,
+    cafs,
+    networkConcurrency: 1,
+    storeDir,
+    verifyStoreIntegrity: true,
+  })
+  expect(typeof requestPackage).toBe('function')
+  const projectDir = tempy.directory()
+
+  {
+    const pkgResponse = await requestPackage({ alias: 'camelcase', pref: `${repo}#${commit}` }, {
+      downloadPriority: 0,
+      lockfileDir: projectDir,
+      preferredVersions: {},
+      projectDir,
+      registry,
+    }) as PackageResponse & {body: {manifest: {name: string}}}
+
+    expect(pkgResponse.body).toBeTruthy()
+    expect(pkgResponse.body.manifest).toBeUndefined()
+    expect(pkgResponse.body.isInstallable).toBeFalsy()
+    expect(pkgResponse.body.id).toBe(`github.com/${repo}/${commit}`)
+  }
+})

packages/prepare-package/src/index.ts

@@ -1,12 +1,12 @@
 import path from 'path'
-import { fromDir as readPackageJsonFromDir } from '@pnpm/read-package-json'
+import { safeReadPackageFromDir } from '@pnpm/read-package-json'
 import rimraf from '@zkochan/rimraf'
 import execa from 'execa'
 import preferredPM from 'preferred-pm'
 
 export default async function preparePackage (pkgDir: string) {
-  const manifest = await readPackageJsonFromDir(pkgDir)
-  if (manifest.scripts?.prepare != null && manifest.scripts.prepare !== '') {
+  const manifest = await safeReadPackageFromDir(pkgDir)
+  if (manifest?.scripts?.prepare != null && manifest.scripts.prepare !== '') {
     const pm = (await preferredPM(pkgDir))?.name ?? 'npm'
     await execa(pm, ['install'], { cwd: pkgDir })
     await rimraf(path.join(pkgDir, 'node_modules'))