tastejs / hacker-news-pwas

HNPWA - Hacker News readers as Progressive Web Apps 📱
https://hnpwa.com
2.38k stars 206 forks source link

Add note to clarify differences in implementations #36

Closed addyosmani closed 7 years ago

addyosmani commented 7 years ago

"HNPWA implementations aim to follow a loose specification. They are primarily a learning tool and should not be used to compare the performance of one PWA to another. They can differ based on server infrastructure, performance patterns used and other factors."

Would that wording (or something a little more terse) work for you? cc @gaearon @rauchg @kristoferbaxter

I am also open to any other feedback on how we can improve the homepage to clarify we're a reference (and not a perf comparison) site.

kristoferbaxter commented 7 years ago

Sounds good to me.

gaearon commented 7 years ago

Even with a note, the website design still makes it seem like a library comparison:

The page was visually designed to look like a performance comparison between libraries rather than an app showcase, and adding text won't help a lot in my opinion, although it is a welcome addition nonetheless.

If each example was a list of bullet points rather than a library title, it would make more sense to me. But it would probably be less useful for drawing quick conclusions based on incomplete data, and thus make the page harder to digest. I don't know enough about the intentions of this campaign, so it is hard for me to say what the best fix would be.

But I think refocusing on app examples, and making UI library just a point in a bullet list including other techniques would help reframe this in a more positive and productive way.

Thanks for listening to feedback!

addyosmani commented 7 years ago

This is all welcome feedback and we appreciate you taking the time to share it.

The goal of the project is to let developers know that it is possible to build PWAs with the frameworks listed that perform well on mobile hardware and highlight through OSS examples some of the ways this can be accomplished. It is not aimed to be a performance comparison tool at all.

I would be happy for us to rethink the whole layout to address the points listed above.

It uses library logo and title instead of example's own logo and title

Initially we can at minimum change the titles. @housseindjirdeh what if we revised the listings so that instead of "React" it said "ReactHN", "Preact" -> "PreactHN" etc?.

If each example was a list of bullet points rather than a library title, it would make more sense to me.

I would personally with happy to change gears in that direction. Good idea. It would also highlight that each implementation differs in build system setup, infra etc.

It doesn't mention anything about build or serving techniques of each examples which can play a bigger role than libraries

We can roll this information into the bullet point breakdown for each library.

@kristoferbaxter Dan also had the idea of us deploying a React version of PreactHN (as ReactHN currently uses quite different patterns). Is that something you might be interested in?

kristoferbaxter commented 7 years ago

I'd love to make a ReactHN using the same basis for PreactHN.

@gaearon is there someone I could ping as I have questions?

gaearon commented 7 years ago

Ping me anytime!

kristoferbaxter commented 7 years ago

Thanks! I'll get started on one over the weekend.

housseindjirdeh commented 7 years ago

@gaearon genuinely can't thank you enough for your feedback. You're right, as much as people are liking this resource there are quite a few people interpreting it as a performance comparison and that was not our intent at all. The blame is on me if the site came across that way because of its design, and I apologize.

Every suggestion you've listed makes perfect sense and I appreciate it, we'll definitely have those changes incorporated in the coming days and week and make sure things are more transparent.

gaearon commented 7 years ago

No worries, thank you so much for response. It’s often hard to predict how people will interpret something until it’s out.

I apologize if I came across as snarky in previous comments! Let me know if there’s any way we can help in the future ❤️

housseindjirdeh commented 7 years ago

Not at all!! Feedback like that is just exactly what we needed, we'll definitely keep you posted as we change things up :)

CaptainCodeman commented 7 years ago

It would be nice if all implementations use the same data-source and displayed the same number of rows for the listings.

While some only show 20 items per page others are showing 30 - which simplistically is 50% more work.

addyosmani commented 7 years ago

It would be nice if all implementations use the same data-source and displayed the same number of rows for the listings.

Let's bake this into the spec. It seems like a low-friction enough addition that just makes sense. I'm in favor of opting for 20 and filing issues against any app using 30 at the moment.

@CaptainCodeman want to file a PR against the README to get your suggestion added in?

kristoferbaxter commented 7 years ago

I published a React port of preact-hn example over the weekend. Will open source the code later this week after resolving an issue with React Router and active states.

https://react-hn.kristoferbaxter.com/

gaearon commented 7 years ago

Nice work!

addyosmani commented 7 years ago

@kristoferbaxter This is fantastic work. Digging into some traces..

3G the main thread settles well before 5s on a Moto G4 https://www.webpagetest.org/video/compare.php?tests=170522_NB_2512e02ac74bc812eb6dd05e7ea21005-r%3A2-c%3A0&thumbSize=200&ival=500&end=full.

EM hits first interactive around 3-4s in and fully settles a few seconds later https://www.webpagetest.org/video/compare.php?tests=170522_RF_469eaf465dcb9b6aeb9eb78e224450b1-r%3A2-c%3A0&thumbSize=200&ival=500&end=full

CaptainCodeman commented 7 years ago

Let's bake this into the spec. It seems like a low-friction enough addition that just makes sense. I'm in favor of opting for 20 and filing issues against any app using 30 at the moment.

Sure, but HackerNews itself seems to use 30 and the most convenient public API mirrors this (https://node-hnapi.herokuapp.com) so should it be the other way round? I'll make a PR with that assumption and can change it if it's decided to go the other way.

I know it won't have a huge impact but the higher number of rows might help separate the results a little by amplifying any slight rendering perf differences.

housseindjirdeh commented 7 years ago

The site has been redesigned since this has been opened to take some really useful suggestions into account (thanks again @gaearon). Will close this ticket now 🙌

Thanks @CaptainCodeman for putting up the PR to add that requirement to the spec :)