Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(Updating out dependencies): Since CI was failing greekeeper kep… #384

Merged
merged 2 commits into from Sep 18, 2018

Conversation

bneumann
Copy link
Collaborator

@bneumann bneumann commented Sep 13, 2018

…t failing too

#381, #365, #364

@sebastianhaas check if you agree. Specifically with those tslint to compiler stuff

@sschmidTU
Copy link
Contributor

This may not be relevant, but with npm install I get some warnings. I've seen the one about tsify before, a bit annoying, because even installing browserify manually doesn't help.

npm WARN eslint-config-standard@12.0.0 requires a peer of eslint-plugin-node@>=7.0.0 but none is installed. You must install
peer dependencies yourself.
npm WARN eslint-config-standard@12.0.0 requires a peer of eslint-plugin-promise@>=4.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN eslint-config-standard@12.0.0 requires a peer of eslint-plugin-standard@>=4.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN karma-webpack@3.0.4 requires a peer of webpack@^2.0.0 || ^3.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN tsify@3.0.4 requires a peer of browserify@>= 10.x but none is installed. You must install peer dependencies yourself

Other than that, i'm all for making greenkeeper happy.

@sebastianhaas
Copy link
Member

Ah good catch we should fix peer dependencies then. Won't work otherwise.

Copy link
Member

@sebastianhaas sebastianhaas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should fix the peer dependency warnings.

@sschmidTU
Copy link
Contributor

sschmidTU commented Sep 14, 2018

By the way, there are also the warnings about skipping optional (OSX) dependencies:

npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.4 (node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.4: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})

Long story short: Don't expect a proper solution from npm.
People have complained to npm for years that this is unnecessary and should be INFO instead of WARN, but npm is apathetic, so you can either use npm install --no-optional, live with the warning, or use yarn instead of npm ;)
npm/npm#11632

@bneumann
Copy link
Collaborator Author

bneumann commented Sep 14, 2018

I tried to solve the peer dependencies but that's not so easy without giving up on some cool plugins. E.g. The HTML plugin for webpack which is needed for our demo relies on the old tapable hooks

@sschmidTU
Copy link
Contributor

If we can't solve peer dependencies, we should merge this, right?

I tried solving peer dependencies with npm-install-peers, but i think actually the packages need to declare the peer dependencies. This seems like a bit of an unpolished concept still in npm.

https://lexi-lambda.github.io/blog/2016/08/24/understanding-the-npm-dependency-model/#Peer_dependency
https://stackoverflow.com/questions/35207380/how-to-install-npm-peer-dependencies-automatically

But i made a PR #394 for this PR that fixes 4 of the 5 warnings.

@bneumann
Copy link
Collaborator Author

As per #394 the dependency clashes are fixed

@bneumann bneumann merged commit f0d1a9c into develop Sep 18, 2018
@sschmidTU sschmidTU deleted the chore/updateDependencies branch September 19, 2018 11:50
sschmidTU added a commit that referenced this pull request Dec 4, 2018
both packages not used in the project anymore since we switched to webpack.
(tsify is a plugin for browserify)

fixes npm audit warnings for browserify

we probably missed this in #384 (adding browserify) and #394 (keeping tsify).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants