Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: take repositoryRoot into account when computing base path with esbuild #754

Merged
merged 3 commits into from Oct 20, 2021

Conversation

eduardoboucas
Copy link
Member

@eduardoboucas eduardoboucas commented Oct 20, 2021

- Summary

When using esbuild, we use as the Lambda base directory whatever value is supplied in basePath, which is the build directory. However, in #739 we introduced the repositoryRoot property, which allows paths outside of basePath to be added to the archive. When this happens, the base path calculation must take both basePath and repositoryRoot into account, using as the final value the common path prefix between the two.

This is currently an issue only when using esbuild with a monorepo setup and modules in externalNodeModules.

- Test plan

I've updated an existing test in dca5597. It adds a second module and adds it to externalNodeModules.

You can see it failing at https://github.com/netlify/zip-it-and-ship-it/pull/754/checks?check_run_id=3949915958, before the fix in 935091a, and then subsequently passing.

- A picture of a cute animal (not mandatory but encouraged)
Llama_antibodies_GettyImages528127648_Thumb-732x549

@eduardoboucas eduardoboucas added the type: bug code to address defects in shipped code label Oct 20, 2021
@eduardoboucas eduardoboucas marked this pull request as ready for review October 20, 2021 09:59
@eduardoboucas eduardoboucas merged commit 8b2f8ea into main Oct 20, 2021
@eduardoboucas eduardoboucas deleted the fix/esbuild-basepath branch October 20, 2021 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants