Conversation
| // encountered this deep into resolution, it's likely because it's an out of | ||
| // repo dependency that was published incorrectly. For example, it may be | ||
| // been mistakenly published with 'npm publish' instead of 'pnpm publish'. | ||
| if (wantedDependency.pref.startsWith('catalog:')) { |
There was a problem hiding this comment.
This is a convenient place to throw an error since the catch block sets up pkgsStack, but let me know if we think this error should be thrown somewhere else.
There was a problem hiding this comment.
maybe do the check only when there's an error.
There was a problem hiding this comment.
I'm wondering if we should reuse existing ERR_PNPM_SPEC_NOT_SUPPORTED_BY_ANY_RESOLVER error instead and detect if the specifier that was unsupported was the catalog: protocol. I realized this might be simpler after looking at this code more today.
I just pushed up this approach as 438d0b9, which removes the throw inside resolveDependencies.ts entirely.
What do you think?
gluxon
force-pushed
the
gluxon/catalog-error-on-external-catalog-usage-better-error
branch
from
622f29c to
438d0b9
Compare
| }) | ||
| return null | ||
| } | ||
| err.package = wantedDependencyDetails |
There was a problem hiding this comment.
One thing I don't like is that these error objects aren't typed, but we could refactor that general problem in a future PR.
|
Thanks for catching those typos. |
zkochan
deleted the
gluxon/catalog-error-on-external-catalog-usage-better-error
branch
2 participants
Problem
I'm predicting a common problem with the
catalog:protocol will be users runningnpm publishinstead ofpnpm publish. I'd like to make errors related to this mistake more clear.Changes
Before
After