fix(zip-it-and-ship-it): create missing symlinks when archiveFormat=n…

…one (#5836)

* fix(zip-it-and-ship-it): create missing symlinks when archiveFormat=none

This change updates zisi's node bundler to create missing symlinks in
when `archiveFormat: 'none'` (the code path used by `netlify dev`).

Currently, in projects where two or more symlinks resolve to the same
target, we only ever create one symlink. In practice, how this usually
presents a problem is when a package manager dedupes a dependency by
symlinking it into two different packages; users end up getting a
runtime error when their code hits the code path that tries to resolve
the missing symlink.

For example, given this scenario (which I took from a project that
exhibits this issue):

```
{
  targetPath: '../../../@netlify[email protected]/node_modules/@netlify/sdk--extension-api-client',
  absoluteDestPath: '/home/ndhoule/dev/src/github.com/netlify/integration-csp/.netlify/functions-serve/trpc/node_modules/.pnpm/@netlify[email protected]_@[email protected]/node_modules/@netlify/sdk--extension-api-client'
}
{
  targetPath: '../../../@netlify[email protected]/node_modules/@netlify/sdk--extension-api-client',
  absoluteDestPath: '/home/ndhoule/dev/src/github.com/netlify/integration-csp/.netlify/functions-serve/trpc/node_modules/.pnpm/@netlify[email protected]_@[email protected]_@[email protected]_@[email protected]._vp3jgimrs3u5kdeagmtefgn5zi/node_modules/@netlify/sdk--extension-api-client'
}
```

...only the second symlink is created. This is because as we walk
through the list of files to output to the build directory, we only
track a single symlink destination per target. Many symlinks can point
at the same target, though, so we need to track multiple symlink
destinations per target.

I tested this out in my problem projects and it solved the problem. I
took a stab at adding a test for this, but got a little lost in the test
setup, which seems to have the `.zip` code path in mind rather than the
`'none'` path. Happy to write some tests if someone can point me at a
test setup.

* test: add regression test for symlink bug

packages/zip-it-and-ship-it/src/runtimes/node/utils/zip.ts

@@ -129,7 +129,7 @@ const createDirectory = async function ({
     addBootstrapFile(srcFiles, aliases)
   }
 
-  const symlinks = new Map<string, string>()
+  const symlinks = new Map<string, Set<string>>()
 
   // Copying source files.
   await pMap(
@@ -156,7 +156,8 @@ const createDirectory = async function ({
       if (stat.isSymbolicLink()) {
         const targetPath = await readLink(srcFile)
 
-        symlinks.set(targetPath, absoluteDestPath)
+        // Two symlinks may point at the same target path, so keep a list of symlinks to create.
+        symlinks.set(targetPath, (symlinks.get(targetPath) ?? new Set()).add(absoluteDestPath))
 
         return
       }
@@ -168,9 +169,11 @@ const createDirectory = async function ({
 
   await pMap(
     [...symlinks.entries()],
-    async ([target, path]) => {
-      await mkdir(dirname(path), { recursive: true })
-      await symlink(target, path)
+    async ([target, paths]) => {
+      for (const path of paths) {
+        await mkdir(dirname(path), { recursive: true })
+        await symlink(target, path)
+      }
     },
     {
       concurrency: COPY_FILE_CONCURRENCY,

packages/zip-it-and-ship-it/tests/fixtures-esm/symlinked-deps/function.mjs

@@ -0,0 +1,6 @@
+export default async () =>
+  new Response('<h1>Hello world</h1>', {
+    headers: {
+      'content-type': 'text/html',
+    },
+  })

packages/zip-it-and-ship-it/tests/symlinked_included_files.test.ts

@@ -1,6 +1,6 @@
 import { readdir } from 'fs/promises'
 import { platform } from 'os'
-import { join } from 'path'
+import { join, sep } from 'path'
 
 import decompress from 'decompress'
 import { dir as getTmpDir } from 'tmp-promise'
@@ -69,6 +69,66 @@ test.skipIf(platform() === 'win32')('Symlinked directories from `includedFiles`
   })
 })
 
+// Regression test for https://github.com/netlify/build/pull/5836
+test('preserves multiple symlinks that link to the same target', async () => {
+  const { path: tmpDir } = await getTmpDir({ prefix: 'zip-it-test' })
+  const basePath = join(FIXTURES_ESM_DIR, 'symlinked-deps')
+  const mainFile = join(basePath, 'function.mjs')
+
+  // Two symlinks that point at `node_modules/.pnpm/[email protected]/node_modules/is-odd`:
+  //
+  // - `node_modules/.pnpm/[email protected]/node_modules/is-odd`
+  // - `node_modules/.pnpm/[email protected]/node_modules/is-odd`
+  expect(await readDirWithType(basePath)).toEqual({
+    'function.mjs': false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even'.replace(/\//g, sep)]: true,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even-or-odd/index.js'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even-or-odd/package.json'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-odd'.replace(/\//g, sep)]: true,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even/index.js'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even/package.json'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-odd'.replace(/\//g, sep)]: true,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-odd/index.js'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-odd/package.json'.replace(/\//g, sep)]: false,
+    ['node_modules/is-even-or-odd'.replace(/\//g, sep)]: true,
+  })
+
+  await zipFunction(mainFile, tmpDir, {
+    archiveFormat: ARCHIVE_FORMAT.NONE,
+    basePath,
+    config: {
+      '*': {
+        includedFiles: ['**'],
+      },
+    },
+    featureFlags: {
+      zisi_fix_symlinks: true,
+    },
+    repositoryRoot: basePath,
+    systemLog: console.log,
+    debug: true,
+    internalSrcFolder: undefined,
+  })
+
+  // Test to be sure we've made both symlinks, not just one of them
+  expect(await readDirWithType(join(tmpDir, 'function'))).toEqual({
+    '___netlify-bootstrap.mjs': false,
+    '___netlify-entry-point.mjs': false,
+    '___netlify-telemetry.mjs': false,
+    'function.mjs': false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even'.replace(/\//g, sep)]: true,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even-or-odd/index.js'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even-or-odd/package.json'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-odd'.replace(/\//g, sep)]: true,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even/index.js'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-even/package.json'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-odd'.replace(/\//g, sep)]: true,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-odd/index.js'.replace(/\//g, sep)]: false,
+    ['node_modules/.pnpm/[email protected]/node_modules/is-odd/package.json'.replace(/\//g, sep)]: false,
+    ['node_modules/is-even-or-odd'.replace(/\//g, sep)]: true,
+  })
+})
+
 test('symlinks in subdir of `includedFiles` are copied over successfully', async () => {
   const { path: tmpDir } = await getTmpDir({ prefix: 'zip-it-test' })
   const basePath = join(FIXTURES_ESM_DIR, 'symlinked-bin')