fix: NPX apply changelog (npx @changesets/cli version)#1562
fix: NPX apply changelog (npx @changesets/cli version)#1562Andarist merged 7 commits intochangesets:mainfrom Netail:fix/npx-apply-changelog
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1562 +/- ##
==========================================
- Coverage 81.02% 81.00% -0.02%
==========================================
Files 54 54
Lines 2229 2232 +3
Branches 662 663 +1
==========================================
+ Hits 1806 1808 +2
- Misses 419 420 +1
Partials 4 4 ☔ View full report in Codecov by Sentry. |
🦋 Changeset detectedLatest commit: 3e44749 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| changelogPath = resolveFrom(changesetPath, config.changelog[0]); | ||
| } catch { | ||
| changelogPath = resolveFrom( | ||
| path.join(__dirname, "..", ".."), |
There was a problem hiding this comment.
I understand what you are doing here but I don't think we should just blindly~ escape the current directory like this in hope to end up in the correct directory. This alternative cwd should be passed down as part of the options to this function
There was a problem hiding this comment.
resolveForm does not seem to accept options.
But isn't cwd the issue here as NPX installs the npm package into a npm cache folder somewhere in the home directory or %localappdata% for windows, while the process itself gets executed in a workspace (potentially) far from this cache directory. Resulting in changeset searching in the local node_modules where the process is executed, while it should search in the npm-cache (close to where the changeset/cli cache is located)
Packages located here:
~/.npm/_npx/<hash>/node_modules/@changesets/cli
~/.npm/_npx/<hash>/node_modules/@changesets/apply-release-plan
~/.npm/_npx/<hash>/node_modules/@changesets/git
Executed here:
~/Documents/React/<project>
__dirname: ~/npm/<hash>/node_modules/@changesets/apply-release-plan/index.js
cwd: ~/Documents/React/<project>There was a problem hiding this comment.
Never mind, I get what you mean. Applied & testing, seems to work :)
| try { | ||
| changelogPath = resolveFrom(changesetPath, config.changelog[0]); | ||
| } catch { | ||
| changelogPath = resolveFrom(__dirname, config.changelog[0]); |
There was a problem hiding this comment.
This is better but this "context dir" should come from options. I feel like we want to resolve this in the context of the executed @changesets/cli and not in the context of @changesets/apply-release-plan, even if that ends up resolving to the same location in the majority of cases.
There was a problem hiding this comment.
Yep, this version works for me 👍 Have you tried testing the current state of this PR in "the real world"?
There was a problem hiding this comment.
Haven't yet since 3c786a6 (Since the __dirname coming from the cli package), before that I did. https://www.npmjs.com/package/@netail/changeset-cli + underlying https://www.npmjs.com/package/@netail/changesets-apply-release-plan
I can check the latest changes tonight
There was a problem hiding this comment.
I can check the latest changes tonight
I'd appreciate it 👍
There was a problem hiding this comment.
Alright, seems to work
ref npx @netail/changeset-cli@2.27.21 version (https://www.npmjs.com/package/@netail/changeset-cli)
…1562) * Resolve module from global node_modules Fixes changesets#566 * fix: NPX apply changelog * fix: feedback * fix: feedback v2 * fix: feedback v3 * Create seven-beans-jump.md * Apply suggestions from code review --------- Co-authored-by: Cefn Hoile <github.com@cefn.com> Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
#1500, but a bit simpler
fixes: #566
fixes: #1426
If you would like to test it yourself;
npx @netail/changeset-cli@2.27.15 version