jedireza / hapi-react-views

:package: A hapi view engine for React components
MIT License
231 stars 33 forks source link

fix: allow peer dep of react 15 or 16 #76

Closed kirbyjs closed 5 years ago

kirbyjs commented 5 years ago

Description

When hapi-react-views updated to react 16, there was nothing that broke support of react@15. This change allows the consumer to use react@15 or greater.

kirbyjs commented 5 years ago

cc @jedireza

kirbyjs commented 5 years ago

Thanks for getting back so quick @jedireza.

I'm not a fan of open ended versioning outside of the current major. If something breaks in a future version of React (v17+), we wouldn't know until someone reported it.

I struggle w/ this as well. I've seen it both ways (>=x.x.x or 1.x.x || 2.x.x) in the JS community. And I don't know if I've reached a conclusion to what my stance for this.

Maybe change the definition to be between >=15.x.x and <=16.x.x?

👍 I'm totally okay w/ this.

Also, you're the first person to bring this up and it's been a long time since we made this change. I'm curious if there's a big upside in supporting v15.

Honestly my goal is just to clean up my npm warnings. I'm not sure how much js devs really care about them.

Also, if hapi-react-views still works w/ react@15 what's wrong w/ saying it does in the package.json? If something gets added that doesn't support react@15 then remove it as a peerDependency and release it as a breaking change.

jedireza commented 5 years ago

Also, if hapi-react-views still works w/ react@15 what's wrong w/ saying it does in the package.json?

Nothin' 😃

If something gets added that doesn't support react@15 then remove it as a peerDependency and release it as a breaking change.

Seems like more effort than upside. The inverse could be said about leaving the warnings in the output (it's not breaking).

I'd like to get a more definitive idea of how long React v15 will be supported. Too bad they don't include an explicit plan like the node project. https://github.com/nodejs/Release

If we're at the tail end of v15's life, I'd rather just skip it.

kirbyjs commented 5 years ago

I'd like to get a more definitive idea of how long React v15 will be supported. Too bad they don't include an explicit plan like the node project. nodejs/Release

I'm not sure they even do support previous releases. Looking at their change log, they've never udpated v15 when v16 came out.

Seems like more effort than upside.

Having to be aware if the latest change breaks support of react@15 can totally be burdensome, I get that.

Like you said, it's not breaking, so I thought it was worth a PR at least. :smile:

jedireza commented 5 years ago

The latest release of v15 was 15.6.2 on November 15, 2017: https://github.com/facebook/react/releases/tag/v15.6.2

I also noticed that semver version ranges don't include pre/alpha releases. I tested by using https://semver.npmjs.com/ and also found this cheat sheet https://devhints.io/semver. (not that this was a deciding factor, just documenting my findings)

When I say it's more work than the potential upside, it's not just merging this PR, it's also versioning and releasing this module to npm.

I appreciate you taking the initiative and creating a PR and discussing all of this. At this point I'm going to pass on this, if it was a year ago I'd probably have gone for it.

I hope this isn't discouraging as a contributor. I found this discourse well worth our time spent.