Skip to content

react as peerDependency #3934

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

Closed
janusch opened this issue Nov 20, 2017 · 5 comments
Closed

react as peerDependency #3934

janusch opened this issue Nov 20, 2017 · 5 comments

Comments

@janusch
Copy link

janusch commented Nov 20, 2017

Q A
Bug or feature request? Causes Bug "Multiple Copies of react installed"
Which Swagger/OpenAPI version? unrelated
Which Swagger-UI version? "swagger-ui": "^3.4.4"
How did you install Swagger-UI? npm install
Which browser & version? all browsers
Which operating system? all

Expected Behavior

declare dep to react as peerDependency not as dependency

Current Behavior

bug if app is using different version of react.

Possible Solution

facebook/react#8026 (comment)

Context

it sneaked in through updating swagger-ui

Would be great if you would not mind changing this.
Thank you for your great work on swagger!

@shockey
Copy link
Contributor

shockey commented Nov 20, 2017

Hi @janusch! Thanks for the report.

Unfortunately, we aren't able to move react/react-dom to peerDependencies, since Swagger-UI is closer to being a JS web application packaged as a library than a React library.

Swagger-UI is commonly used in non-React contexts. As an example, here's Swagger-UI inside of an Angular 4 app: https://github.com/shockey/swagger-ui-angular4/blob/master/package.json#L26

You do raise a valid point w/r/t React 16. Perhaps we could declare React as a dependency and a peer dependency, so that downstream react 16 modules satisfy one of the requirements,, but I can't find any npm documentation for this use case.

I'll look into this some more and circle back to this thread.

@janusch
Copy link
Author

janusch commented Nov 22, 2017

Hi @shockey,
thank you for the quick reply. I understand your point of view.
I think it is a better trade off to install react after npm warns for missing peerDependency of swagger-ui instead of introducing possible bugs that are first caught on runtime. Sadly I am not aware of a better solution for this. In the project you link to, react could be added to the packages and npm would not warn about missing peer dependency and hings should just work.

In my case, i think ill pull out swagger-ui of the project and use it in a separate.

Thank you for looking into this!

@arsdehnel
Copy link

Haven't done a ton of npm package publishing but wondering if it's possible to create two packages for this? Leave the existing one as it is that includes React as dependency but then include another one that has it as a peer? Seems weird but would solve a problem that anyone using React is going to have at one point or another and troubleshooting it can be very confusing.

For right now is the recommended approach to fork this, move React to peerDep and then use our fork as the package name in our package.json?

@shockey
Copy link
Contributor

shockey commented May 15, 2018

wondering if it's possible to create two packages for this?

Yeah, this is possible - we're already doing that with swagger-ui-dist, which is also generated from this repository.

@shockey
Copy link
Contributor

shockey commented Mar 2, 2019

Fixed!

We've just released swagger-ui-react, which declares React as a peer dependency.

@shockey shockey closed this as completed Mar 2, 2019
@lock lock bot locked and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants