duaraghav8 / solium-plugin-security

The Official Security Plugin for Ethlint (formerly Solium)
http://npmjs.com/package/solium-plugin-security
MIT License
44 stars 12 forks source link

npm warning requiring peer of solium #33

Open zachlysobey opened 5 years ago

zachlysobey commented 5 years ago

When installing ethlint you get a npm warning message:

npm WARN solium-plugin-security@0.1.1 requires a peer of solium@^1.0.0 but none is installed. You must install peer dependencies yourself.

I expect this is because solium has now been renamed to ethlint, but this project still lists solium as a peer dependency

  "peerDependencies": {
    "solium": "^1.0.0"
  },

https://github.com/duaraghav8/solium-plugin-security/blob/master/package.json#L29-L31

I'm happy to open a PR to address this, but I'm not super familiar with how peerDependencies work.

I think ideally it'd specify that it could have a peer depednecy of solium@^1.0.0 OR ethlint@^1.0.0?

duaraghav8 commented 5 years ago

Great point @zachlysobey I just confirmed from docs and the actual code that there is unfortunately no way to specify an OR condition in peer deps.

I'll open up an issue with NPM. Until an OR is possible, I'll add this caveat to the Docs.

I don't want to change the peer dep from solium to ethlint right now because it could be breaking for anyone who treats warnings as errors too.

I'm open to hearing any suggestions you have on solving this problem. Please ignore the warning for now.

duaraghav8 commented 5 years ago

(This comment is for my own future reference)

Issue has been added to Blocked Tasks.

Once OR is available:

duaraghav8 commented 5 years ago

Discussion opened at https://npm.community/t/allow-any-one-of-specified-packages-in-peerdependencies/4933

pcowgill commented 5 years ago

@duaraghav8 What about renaming this repo to ethlint-plugin-security and publishing to npm from the main feature branch a new package named ethlint-plugin-security with a peer deep of ethlint, and from a legacy feature branch publishing the old package named solium-plugin-security with the existing peer dep?

pcowgill commented 5 years ago

@duaraghav8 What about renaming this repo to ethlint-plugin-security and publishing to npm from the main feature branch a new package named ethlint-plugin-security with a peer deep of ethlint, and from a legacy feature branch publishing the old package named solium-plugin-security with the existing peer dep?

Do you think this would be a workable solution? Thanks!

duaraghav8 commented 5 years ago

Hey @pcowgill sorry for late response, yes this is the ideal solution, but unfortunately I don't have the bandwidth to change this, because this requires huge changes in this repo as well as some changes & tests in core ethlint (to allow reading npm modules prefixed with ethlint-plugin-, currently it can only read solium-plugin-)

pcowgill commented 5 years ago

@duaraghav8 Totally understandable. Thanks for getting back to me!