Repository navigation
ci: install the packed tarball itself in the entry-point smoke test - #227
Conversation
The step extracted the tarball and ran `npm install ./package`. npm installs a local folder as a symlink and installs that folder's devDependencies along with it: vitest, vite, the dts plugin, about 800 packages. So the test loaded the library in an environment no consumer has, where a missing runtime dependency could have been satisfied by a dev one. It also made the step fragile. npm crashed resolving that tree on the vitest 4 dependabot PR (#222) with `Cannot read properties of null (reading 'edgesOut')`, while the library and its tests were fine. Installing the .tgz directly gives a plain copy with react, react-dom and nothing else. A check that node_modules/react-xarrows is not a symlink keeps it that way. Co-Authored-By: Claude <noreply@anthropic.com>
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
✅ Deploy Preview for react-xarrows ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to CI now tests the packed package in a clean consumer install, reducing the chance that development dependencies hide missing package contents. No concrete merge-blocking risk remains. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: the CI smoke test now installs the packed tarball directly. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
2 participants
The entry-point smoke test extracted the tarball and ran
npm install ./package. npm installs a local folder as a symlink and also installs that folder's devDependencies. In this case that meant vitest, vite and the dts plugin, about 800 packages.That causes two problems:
Cannot read properties of null (reading 'edgesOut'). That is the only failing check on build(deps-dev): bump vitest from 3.2.7 to 4.1.11 #222. Its tests, type tests and build all pass.This PR installs the
.tgzdirectly, which gives a plain copy with onlyreactandreact-domnext to it. Atest ! -Lcheck fails the step if the package is ever linked again.Evidence
Run locally against a fresh
pnpm packof main:npm install ./package(before)npm install <tgz>(after)npm ls --alllinesnode_modules/react-xarrowsThe "after" column ran the two edited steps verbatim from this branch's
ci.yml, with only the/tmppaths changed.After this merges,
@dependabot rebaseon #222 should turn it green. I merged #222 onto current main locally and lib tests, example tests,test:typesand the smoke step all pass on vitest 4.1.11.🤖 Generated with Claude Code
Summary by CodeRabbit