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

Removes framework usage section #592

Merged

Conversation

@housseindjirdeh
Copy link
Contributor

housseindjirdeh commented Dec 10, 2019

A number of these frameworks aren't being detected 100% correctly and many missed detections are skewing the results.

I'm currently working on making sure the detections in place are accurate so will bring this section back once that's complete.

@housseindjirdeh housseindjirdeh requested a review from rviscomi Dec 10, 2019
Copy link
Contributor

bazzadp left a comment

@housseindjirdeh I don’t think we should remove these sections completely. As you are aware they have already been widely shared and to just remove them without comment feels wrong.

I think instead we should leave the heading in place and replace the text with wording to the effect of “A previous version of this chapter discussed the usage of frameworks. However, we are investigating queries around the accuracy of these statistics and have decided to remove this section temporarily while we complete this review.” Or something like that.

Also can you update the last_updated date in the meta data at the top? Ideally you would run npm run generate too if possible and include the generated HTML and site maps in this PR.

Also do you feel this is important enough to warrant breaking our self-imposed change freeze (see #488)?

@rviscomi

This comment has been minimized.

Copy link
Member

rviscomi commented Dec 10, 2019

Could you elaborate on which detections are incorrect? We should also open issues against the Wappalyzer repo to get this fixed upstream.

I'm hesitant to cut out content before understanding what's wrong.

@bazzadp bazzadp added this to TODO in Web Almanac via automation Dec 11, 2019
@housseindjirdeh

This comment has been minimized.

Copy link
Contributor Author

housseindjirdeh commented Dec 17, 2019

@bazzadp thanks, updated 🚀

And definitely not critical! This can easily go in after the change freeze is done :)

@housseindjirdeh

This comment has been minimized.

Copy link
Contributor Author

housseindjirdeh commented Dec 17, 2019

@rviscomi Most detections in wappalyzer (React, Vue, etc...) rely on global vars which mean bundled apps that don't expose them aren't detected correctly. There's an open issue for this.

Switching wappalyzer's detection model to relying on DOM checks (via a Treewalker for example) would probably take some time which is why I've begun relying on the Library Detector extension since we have admin access to that.

I can definitely push for updating how frameworks get detected in Wappalyzer and hopefully we can see that movement there. In the meantime, would you be okay if we rely on querying against the library detector extension for the results in this chapter?

Copy link
Contributor

bazzadp left a comment

Will leave @rviscomi to decide on this one, but made a couple of suggestions to style as an aside, and to keep figure links the same. Should npm run generate again with this change to restore the figure numbers.

Although there has been a shift towards a component-based model, many older frameworks that follow the MVC paradigm ([AngularJS](https://angularjs.org/), [Backbone.js](https://backbonejs.org/), [Ember](https://emberjs.com/)) are still being used in thousands of pages. However, [React](https://reactjs.org/), [Vue](https://vuejs.org/) and [Angular](https://angular.io/) are the most popular component-based frameworks ([Zone.js](https://github.com/angular/zone.js) is a package that is now part of Angular core).

Although this analysis is interesting, it's important to keep in mind that these results rely on a third-party detection library - [Wappalyzer](https://www.wappalyzer.com). All these usage numbers depend on the accuracy of each [detection mechanism](https://github.com/AliasIO/Wappalyzer/blob/master/src/apps.json) in place.
Note: A previous version of this chapter discussed the usage of frameworks in this section. However, we are investigating queries around the accuracy of these statistics and have decided to remove this section temporarily while we complete this review.

This comment has been minimized.

Copy link
@bazzadp

bazzadp Dec 17, 2019

Contributor
Suggested change
Note: A previous version of this chapter discussed the usage of frameworks in this section. However, we are investigating queries around the accuracy of these statistics and have decided to remove this section temporarily while we complete this review.
<aside class="note">Note: A previous version of this chapter discussed the usage of frameworks in this section. However, we are investigating queries around the accuracy of these statistics and have decided to remove this section temporarily while we complete this review.</aside>
<!-- Empty figure to keep figure numbers the same
<figure>
</figure>
-->
@housseindjirdeh housseindjirdeh force-pushed the housseindjirdeh:removes-framework-section branch from 5f259d5 to 564e0c0 Jan 2, 2020
@housseindjirdeh

This comment has been minimized.

Copy link
Contributor Author

housseindjirdeh commented Jan 2, 2020

@bazzadp thank you! And good catch on keeping the figure numbers consistent.

@rviscomi Will also defer to you. If you would still like to keep the current text instead of removing it altogether, then that's fine! This section seems to be the most popular section on the chapter hence my hesitation to keep things as they are :)

@rviscomi

This comment has been minimized.

Copy link
Member

rviscomi commented Jan 6, 2020

I'd propose the following:

  1. Add a paragraph to the Wappalyzer section of the Methodology describing the limitations of library detection.
  2. Add a note to the JS chapter linking to the Wappalyzer section of the Methodology and summarizing how the limitations may affect our analysis.
  3. Continue to push for a solution in AliasIO/wappalyzer#2450

As I mentioned to @housseindjirdeh in a DM: "We do our best to report on the state of the web given the tools available to us. If they have blind spots, we should note them but not necessarily discount their findings."

@housseindjirdeh housseindjirdeh force-pushed the housseindjirdeh:removes-framework-section branch from 3f1f752 to c0becd9 Jan 7, 2020
@housseindjirdeh

This comment has been minimized.

Copy link
Contributor Author

housseindjirdeh commented Jan 7, 2020

Sounds good @rviscomi! Added a bolder note at the beginning of the section to emphasize these results aren't entirely accurate

CC @StephenFluin

@@ -289,24 +289,24 @@ Other top used JavaScript libraries include jQuery variants (jQuery migrate, jQu

### Frameworks and UI libraries

<aside class="note">Although interesting, the results in this section are not accurate. As mentioned in our <a href="./methodology#wappalyzer">methodology</a>, the third-party detection library used in HTTP Archive (Wappalyzer) has a number of limitations with regards to how it detects certain tools. For JavaScript libraries and frameworks, the limitations are described in more detail <a href="https://github.com/AliasIO/wappalyzer/issues/2450">here.</a></aside>

This comment has been minimized.

Copy link
@rviscomi

rviscomi Jan 7, 2020

Member

Rather than suggesting that the results are inaccurate, which I would dispute, I think all that needs to be acknowledged is that there's a known limitation or blind spot in the Wappalyzer library and these numbers should be taken as a lower bound.

All of our tools have blind spots: HA only measures home pages, CrUX excludes SPAs, Lighthouse only measures on mobile, WebPageTest only runs in Chrome, Wappalyzer only looks at global JS, etc. We acknowledge these limitations on the Methodology page but we haven't dissuaded readers from trusting the results.

This comment has been minimized.

Copy link
@housseindjirdeh

housseindjirdeh Jan 9, 2020

Author Contributor

I decided to mention that because for this specific scenario, the blind spot may actually be quite large (which can affect the end result of "which framework is more popular than the rest").

But good point, there are many limitations with the tools used in the entire almanac. Removed the sentence 👍.

@rviscomi rviscomi merged commit 1dc5467 into HTTPArchive:master Jan 9, 2020
1 check passed
1 check passed
calibreapp/image-actions
Details
Web Almanac automation moved this from TODO to Done Jan 9, 2020
Copy link
Contributor

bazzadp left a comment

Know this one has been round the houses but can I suggest one more change?

@@ -289,24 +289,24 @@ Other top used JavaScript libraries include jQuery variants (jQuery migrate, jQu

### Frameworks and UI libraries

<aside class="note">As mentioned in our <a href="./methodology#wappalyzer">methodology</a>, the third-party detection library used in HTTP Archive (Wappalyzer) has a number of limitations with regards to how it detects certain tools. For JavaScript libraries and frameworks, the limitations are described in more detail <a href="https://github.com/AliasIO/wappalyzer/issues/2450">here.</a></aside>

This comment has been minimized.

Copy link
@bazzadp

bazzadp Jan 9, 2020

Contributor

Nit: Can we change links to Markdown?

Also can we have a better link than "here" (plus full stop was in wrong place).

What about something like this?:

Suggested change
<aside class="note">As mentioned in our <a href="./methodology#wappalyzer">methodology</a>, the third-party detection library used in HTTP Archive (Wappalyzer) has a number of limitations with regards to how it detects certain tools. For JavaScript libraries and frameworks, the limitations are described in more detail <a href="https://github.com/AliasIO/wappalyzer/issues/2450">here.</a></aside>
<aside class="note">As mentioned in our (methodology)[./methodology#wappalyzer"], the third-party detection library used in HTTP Archive (Wappalyzer) has a number of limitations with regards to how it detects certain tools. There is an open issue [to improve detection of JavaScript libraries and frameworks](https://github.com/AliasIO/wappalyzer/issues/2450), which will have impacted the results presented here.</aside>
@bazzadp

This comment has been minimized.

Copy link
Contributor

bazzadp commented Jan 9, 2020

Ah too late. Ah well never mind.

rviscomi added a commit that referenced this pull request Jan 9, 2020
@rviscomi

This comment has been minimized.

Copy link
Member

rviscomi commented Jan 9, 2020

Sorry. Let's fix in another PR.

@housseindjirdeh

This comment has been minimized.

Copy link
Contributor Author

housseindjirdeh commented Jan 10, 2020

Thanks @bazzadp, PR #624 to update the second link

Nit: Can we change links to Markdown?

I don't think markdown links are supported within HTML elements. Also noticed that there are some chapters that are doing this and aren't rendering links properly.

I can try to update all the broken links in the almanac when I get the chance 🙏

@bazzadp

This comment has been minimized.

Copy link
Contributor

bazzadp commented Jan 10, 2020

Nit: Can we change links to Markdown?

I don't think markdown links are supported within HTML elements. Also noticed that there are some chapters that are doing this and aren't rendering links properly.

Oh good point! Forgot I asked you to put is in an <aside>. Thanks for all the changes - that other one's merged now.

I can try to update all the broken links in the almanac when I get the chance 🙏

Oh oh. Going to have to do a grep for this and correct ASAP before my OCD explodes!!! 😊

@bazzadp

This comment has been minimized.

Copy link
Contributor

bazzadp commented Jan 10, 2020

I can try to update all the broken links in the almanac when I get the chance 🙏

Oh oh. Going to have to do a grep for this and correct ASAP before my OCD explodes!!! 😊

Did a grep and that's the only one I can find wrong so submitted #627 for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Web Almanac
  
Done
3 participants
You can’t perform that action at this time.