search

Yomguithereal / react-blessed

A react renderer for blessed.
MIT License
4.45k stars 177 forks source link

Fixed devtools #105

Closed lins05 closed 3 years ago

lins05 commented 4 years ago

Current devtools code is a bit out of date.

Also:

Yomguithereal commented 4 years ago

Hello @lins05, this looks good to me. However why:

Put "ws" and "react-devtools-core" to "devDependencies", which IMO is better than "peerDependencies"

Why is that so? Doesn't this mean that anyone installing the lib even without needing the devtools will get those then useless packages?

iamdustan commented 4 years ago

I had the understanding that peerDependencies was to inform consumers of the library that they would need to install those. Since devDependencies are only installed while working no this library directly users won’t get them or get the peerDep notification that they need to install react-devtools-core to use react-devtools in their apps.

lins05 commented 4 years ago

+1 for what @iamdustan says. "devDependencies" won't affect end users. It would only affect people work on the react-blessed project directly, in which case it would be actually even more handy to have them.

Yomguithereal commented 4 years ago

Sorry, I read your devDependencies as dependencies, hence my interrogation. However, I read @iamdustan as advocating for keeping those in peerDependencies, no?

iamdustan commented 4 years ago

Yeah, that was my intention @Yomguithereal. My intent was to keep them in peerDependencies for consumers. It may make sense to put them in both peerDependencies and devDependencies to serve both audiences (react-blessed contributors and consumers)

Yomguithereal commented 3 years ago

I have reworked this. The devtools have been upgraded and are now totally and safely optional. One only needs to install react-devtools-core for it to work as intended. Maybe I should add something to the docs in this regard?