Comments (10)
Thanks for the issue - this seems like a valid use case and I've started looking into it.
from install-peerdeps.
Thanks so much for your help. Let me know if there's anyway I can assist.
from install-peerdeps.
Any progress on this issue? Is there anyway I can help?
I'd like to update the peer dependencies for my library data-forge-indicators, although then you'll lose my test case. Will that be a problem or do you want me to hold out from updating it a bit longer?
from install-peerdeps.
Hi, thanks for reaching out again. I haven't been able to work on it recently - if you could submit a pull I'll gladly accept it.
One potential solution could be reading the package.json
in __dirname
(i.e. location where install-peerdeps
is being run) and comparing the package.json
version to the peerdep requirement using semver
.
Other thoughts:
- If one version cannot satisfy different constraints, prompt user for desired version
from install-peerdeps.
I've started having a look into this problem. I've written a test (added to cli.test.js) that replicates the problem, and @nathanhleung I'd like your thoughts on it:
test("peer dependency is not installed when a later version already exists", t => {
//TODO: Remove existing node_modules under fixture "has-newer-peer-dep".
//TODO: Do an npm install "has-newer-peer-dep".
const cli = spawnCli("data-forge-indicators", "has-newer-peer-dep");
cli.on("exit", code => {
fs.readFile(
"fixtures/has-newer-peer-dep/node_modules/data-forge/package.json",
"utf8",
(err, packageFileJson) => {
if (err) {
t.fail((err && err.stack) || err);
t.end();
return;
}
t.equal(code, 0, `errored, exit code was ${code}`);
const packageFile = JSON.parse(packageFileJson);
t.equal(packageFile.version, "1.3.3", "Bad version!");
t.end();
}
);
});
});
Some problems:
- There is currently some manual setup required to run the test. Before running the test I change directory into the new fixture directory has-newer-peer-dep, then I delete the node_modules directory and do an npm install to get it into a known start state. Ideally this would be part of the test (see TODO comments above). Although it feels wrong. Should I be mocking this somehow instead of having the test interact directly with the file system?
- I'm actually using data-forge and data-forge-indicators in the test. I did this because I wanted to be sure I was replicating the exact same problem that I'm having, although it feels a bit wrong to be working with real npm modules. Do you have any technique for mocking npm libraries for testing?
Once I've figured out the correct way to structure this test it shouldn't be too hard too actually implement code to pass the test (I think writing the test is probably more difficult than the code for the actual feature).
from install-peerdeps.
Hi @ashleydavis and thanks for the test!
-
The
fixtures
directly is there for the purposes of testing, so no worries about mocking the file system. One thing you can do, however, to automate the manual set-up, is use Node's nativespawn
imported fromchild_process
and pass thecwd
(like you did above) with the commandrm -rf node_modules
. You can runnpm install
viaspawn
as well. -
That's fine, and all the better if you fix your own issue! Although from a philosophical standpoint we could argue that the tests should be pure, in my view the fact that the tool is inherently side-effect-ful (making network requests, writing to the filesystem, etc) means that if our tests have side effects and work with real-world, potentially changing data, that's OK too (of course, if
data-forge
is removed from NPM we have a problem, but we can easily fix it by choosing a different module).
Appreciate your work so far and hope my comments are clear. Let me know if you've got any questions!
from install-peerdeps.
@nathanhleung I've committed the new test and code to make it pass.
Unfortunately I've broken several other tests and I'm having a hard time figuring out why! Do you mind having a look and maybe pointing me in the right direction?
Thanks!
from install-peerdeps.
Thank you for your contributions so far, and apologies for the late reply!
I've created a pull request with your latest changes so they could run on Travis. It looks the tests that add the global
and save
flags are the ones that are failing. I'm not exactly sure why either, but I've added some comments to the pull request (#46) which might help.
Also, I'm not sure if you can add commits to #46 yourself since I created it. If that's the case, feel free to open a new pull request and I'll close the one I created.
Thanks again for your help!
from install-peerdeps.
Hey, I think I'm going to park this for now. It's turned out to be a bigger job than I expected. I realised today that it's actually much simpler for me replicate the small portion of install-peerdeps taht that I need into my own project than it is to try and get my contribution to install-peerdeps working properly.
Thanks again for this library, sorry that I don't have more time to contribute to it right at the moment. Maybe I'll come back to it again the future.
from install-peerdeps.
No worries, thank you for your help so far and I'll keep this open in case you want to come back to it!
from install-peerdeps.
Related Issues (20)
- 'request' package needs to be replaced
- ERR spawn npm ENOENT HOT 21
- Fix issue with `child_process.spawn` on Windows
- Move tests to GitHub Actions, add Windows Tests HOT 11
- `ERR undefined` with v3.0.1 when running on Windows 10 HOT 5
- Feature request: Install from package.json
- Do we have the actual source code for this tool? HOT 3
- Cannot read property 'forEach' of undefined HOT 8
- should not error when there are no peer dependencies; just quietly install the package and move on HOT 3
- ERR: undefined when installing eslint-config-airbnb HOT 7
- --no-registry flag does not work HOT 2
- ERR Unexpected end of JSON input HOT 6
- `@babel/polyfill` warning HOT 3
- Install peer deps using absolute url
- `ERR undefined` with v3.0.3 install-peerdeps HOT 4
- Feature Request: Add ability to ignore some peerDependencies HOT 4
- load package version failure when using yarn@3
- --no-registry not work HOT 1
- install-peerdeps `ERR undefined` on windows HOT 6
- Installing with Yarn via `yarn dlx` or `npx` ends in success with no package.json updates or changes
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from install-peerdeps.