Skip to content
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

Ported to Preact #235

Closed
Download opened this issue Feb 8, 2017 · 7 comments
Closed

Ported to Preact #235

Download opened this issue Feb 8, 2017 · 7 comments

Comments

@Download
Copy link

Download commented Feb 8, 2017

Just a heads up for you guys that I created a port of this library for Preact:

preact-helmet

@potench
Copy link
Contributor

potench commented Feb 8, 2017

Would it make more sense to use "preact-compat" instead of maintaining a fork specific for Preact? https://github.com/developit/preact-compat

@Download
Copy link
Author

Download commented Feb 8, 2017

Maybe. But I'm not using preact-compat and I didn't want to add it just for Helmet :)
I don't expect there will be more major changes to this library, but maybe that will prove to be a wrong call on my part :)

@cwelch5
Copy link
Contributor

cwelch5 commented Feb 10, 2017

@Download We do have plans for modularizing the different head types (meta, link, etc.) into their own components, as well as bug fixes and refactoring. It would be great to have this in preact-compat instead. But for now, could you remove the NFL Engineers logo from your README and any references to NFL as we'd like to make it clear we aren't supporting that fork. We appreciate it.

@cwelch5 cwelch5 closed this as completed Feb 10, 2017
@Download
Copy link
Author

Ok I will...

But:

There are 142 forks out there... Do you ask all of them to remove the logo (it is just there in the README right after you fork it) ?
I must admit I had expected a more supportive response than this.... :(

Download added a commit to Download/preact-helmet that referenced this issue Feb 11, 2017
@Download
Copy link
Author

It would be great to have this in preact-compat instead.

I disagree. The whole idea of preact-compat is that you can use it to adapt Preact so you can use the original React-compatible component. It would defeat the purpose of porting react-helmet in the first place. You should be able to use react-helmet unchanged with preact using preact-compat... so what would be the point of a fork?

The whole reason for the fork is so people (mostly, myself) can use react helmet without needing preact-compat.

@cwelch5
Copy link
Contributor

cwelch5 commented Feb 16, 2017

@Download You make a good point, if you don't see the need to use preact-compat for any other libs then you could save a lot of bloat. It does offer more flexibility for people that may just want to use Helmet with Preact.

I think we were suggesting preact-compat originally because most people who buy-in to the whole Preact ecosystem would most likely use it to give themselves access to all their familiar libs, without having to find converted versions of all of them or maintain them themselves. It's just more maintenance on your part, but of course it's great to see Helmet being extended and ported and utilized in more places, so definitely can't fault you for that.

Regarding the README, yes, you are right, everyone gets the exact same README for their fork which includes our logo and verbiage, but I can't recall seeing anyone really changing it at all, and if they do, they remove the logo and just state the relevant info for their fork. You went to the trouble of changing all instances of React to Preact, which is fine, totally makes sense, but that is a significant change and may cause confusion to someone on the outside. I'm not sure the legalities around it though, it was just a friendly suggestion, sorry if it didn't come off that way. Ah, I see, when I stated in my last response "not supporting that fork", I didn't mean that we were against it, just that we don't maintain it. I hope that's clear.

@Download
Copy link
Author

@cwelch5
Hi Chris thanks for clearing that up!

when I stated in my last response "not supporting that fork", I didn't mean that we were against it, just that we don't maintain it.

Probably if we would have had this conversation in person, I wouldn't have taken it the wrong way... but I did. I felt hurt. In re-reading your message after your explanation, I suddenly see how it can be read in a completely different way! Sorry if I acted but-hurt.. but I was.

Thanks for your kind words I feel a lot better now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants