netlify / build

Netlify Build (node process) runs the build command, Build Plugins and bundles Netlify Functions. Can be run in Buildbot or locally using Netlify CLI
https://docs.netlify.com/configure-builds/build-plugins/
MIT License
218 stars 57 forks source link

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

Closed ndhoule closed 2 months ago

ndhoule commented 2 months ago

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+sdk--extension-api-client@2.2.0/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+sdk--ui-functions@1.0.4_@trpc+server@11.0.0-rc.477/node_modules/@netlify/sdk--extension-api-client'
}
{
  targetPath: '../../../@netlify+sdk--extension-api-client@2.2.0/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+sdk@2.3.1_@google-cloud+storage@5.20.5_@trpc+server@11.0.0-rc.477_@types+react@18.3._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.

github-actions[bot] commented 2 months ago

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files. Consider converting them to TypeScript.

ndhoule commented 2 months ago

Added a test.

Not sure why this macOS test is timing out, but it seems unrelated to my changes.