-
Notifications
You must be signed in to change notification settings - Fork 35
feat: pnpm-style symlinking (#636) #657
Conversation
⏱ Benchmark resultsComparing with b105692 largeDepsEsbuild: 13s⬆️ 4.41% increase vs. b105692
Legend
largeDepsZisi: 1m 11.4s⬆️ 5.81% increase vs. b105692
Legend
|
@netlify-team-account-1 what's the status of this one? |
This is the test that was originally added a while ago to reproduce #636 (the issue we paired on). We found that this reproduction doesn't correctly capture the real issue - but the test may still be useful to prevent future bugs, so why not merge it. |
Sounds good! |
opts: { config: { '*': { nodeBundler: bundler } } }, | ||
}) | ||
|
||
// eslint-disable-next-line import/no-dynamic-require, node/global-require |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to remove these once we land #682.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#682 is only about fixtures, not about test code. We could add // eslint-disable-file import/no-dynamic-require, node/global-require
to the top of this file instead of line-by-line, though!
Curious if this also solves for this issue with regular |
Not sure what issue you're referring to - if there's already an issue for it in this repository, could you link it? If not, feel free to open one :) |
…/zip-it-and-ship-it#657) * chore: add repro test for pnpm-style symlinking * chore: show that it works with esbuild-based bundlers Co-authored-by: Netlify Team Account 1 <netlify-team-account-1@users.noreply.github.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Contains a reproduction test for pnpm-style symlinking, reported non-working in #636.