Skip to content
This repository has been archived by the owner on Mar 3, 2022. It is now read-only.

Remove babel-polyfill from peerDependencies #535

Closed
fttriquet opened this issue Apr 25, 2018 · 13 comments
Closed

Remove babel-polyfill from peerDependencies #535

fttriquet opened this issue Apr 25, 2018 · 13 comments
Assignees
Milestone

Comments

@fttriquet
Copy link

fttriquet commented Apr 25, 2018

#Can babel-polyfill be removed from peerDependencies ?

In our app we do not need that dependency to use oidc client, but removing it from our dependencies will cause the unmet peer dependency warning.

cc @davinkevin

@brockallen brockallen self-assigned this May 4, 2018
@brockallen brockallen added this to the 1.5.0 milestone May 4, 2018
@brockallen
Copy link
Member

Yes, I was thinking of how to build this with two versions -- one with the polyfill and one without. I'll see what I can do.

@brockallen
Copy link
Member

So this is the PR that added it: #166

So should we have it or not? // @kristofdegrave

@brockallen
Copy link
Member

@fttriquet any update?

@fttriquet
Copy link
Author

Sorry for the delay, nothing more on my side, having a specific version without that dependency would do the trick for us.

@brockallen
Copy link
Member

As I see it, you might need this peerDependency... it depends on what and how you deploy your app.

@davinkevin
Copy link

It could be an optional dependency ?

@brockallen
Copy link
Member

Yes, depending on how you use/package/depend on oidc-client. If you use it as a ES6 module or use the UMD in ~/lib and run in an older browser then you need to have the polyfill in there somehow. That was the point of the PR from @kristofdegrave, I believe.

@fttriquet
Copy link
Author

I see what you mean, but it seems to be quite a specific case their, I would say it's up to the client to manage that part ? Kevin's proposition would solve this (without having to do several builds), and add en entry in the readme about polyfill.

@davinkevin
Copy link

@brockallen The PR of @kristofdegrave add it to peerDependencies instead of optionaldependencies (docs)

Putting it inside this here should solve the original problem of @fttriquet and won't break because peerDependencies are not installed since NPM 3 (& Yarn)

@brockallen
Copy link
Member

Ah I wasn't aware of the dedicated optional dependency config (which explains why I ignored the mention of it above). That sounds like the right place. Thanks

@brockallen
Copy link
Member

Done, thanks for helping me thru that!

@davinkevin
Copy link

Thx 👍

@fttriquet
Copy link
Author

Great news, thank you

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

3 participants