strongloop / loopback-component-passport

LoopBack passport integration to support third party logins and account linking
Other
139 stars 228 forks source link

Add loopback peer dependency #252

Closed jt-nti closed 4 years ago

jt-nti commented 6 years ago

Description

Add loopback peer dependency

Related issues

Missing loopback dependency #245

Checklist

virkt25 commented 6 years ago

@slnode ok to test

virkt25 commented 6 years ago

@jt-nti Thanks for opening the PR! Much appreciated :)

Any reason to not make it a devDependency or direct dependency?

cc: @raymondfeng you might be more familiar with the module, can you suggest what type of dependency loopback should be?

jt-nti commented 6 years ago

@virkt25 adding it as a direct dependency was my first instinct however PR #246 was closed by @raymondfeng because applications using loopback are expected to include the dependency. A peer dependency seems to be more appropriate in this case, which should be in addition to a dev dependency rather than replacing it since the tests also require loopback.

virkt25 commented 6 years ago

Is there a use case where you want to use this component outside of a loopback project? I personally don't mind the explicit declaration but none of the LoopBack components / connectors declare loopback as a peerDependency / regular dependency?

jt-nti commented 6 years ago

@virkt25 I was encountering problems trying to use loopback in a rush monorepo, which seemed to work when there was an explicit dependency

virkt25 commented 6 years ago

Adding it as a peerDependency just make the dependency explicit instead of implicit ... but doesn't solve the problem as the user will still have to install the dependency (npm > 2 just warns and doesn't install peer dependencies).

I think it might be best to declare the peerdependency / devDependency within the package using this component within your monorepo? ... Thoughts @jt-nti ?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.