allow fetch as request - Githubissues

simonbesters commented 7 months ago

Vermoedelijk is wappa om performance-redenen beperkt in requests types. React websites van SIM gebruiken fetch om vanuit main.js (geladen vanaf 3rd party / cdn) om alle resources te laden, zie:

https://www.ameland.nl / https://www.achtkarspelen.nl

Door in driver.js naast 'document' en 'script' ook 'fetch' toe te staan zorg je dat de requests niet geblokkeerd worden en zo de verschillende Analytics-tools geladen worden.

driver.js regel 658:

if (
          (responseReceived && request.isNavigationRequest()) ||
          request.frame() !== page.mainFrame() ||
          !['document', ...(this.options.noScripts ? [] : ['script']), 'fetch'].includes(
            request.resourceType()
          )
        )

Op deze manier lukt het wel. Is een tijdelijke oplossing, want misschien moeten we de wappa efficiëntie een stukje terugschroeven om betere resultaten te realiseren.

rudiedirkx commented 7 months ago

Dat is niet echt leesbaar, hoe je het hebt aangepast :D Het was al totaal niet leesbaar in Wappa, maar je hebt het NOG onleesbaarder gemaakt 😆

simonbesters commented 7 months ago

Dat is niet echt leesbaar, hoe je het hebt aangepast :D Het was al totaal niet leesbaar in Wappa, maar je hebt het NOG onleesbaarder gemaakt 😆

Haha. Ja, had hem beter vooraan erbij kunnen zetten en niet achter de optie. Maar dat is voor de grote opruiming als we echt niks meer te doen hebben...

rudiedirkx commented 7 months ago

Op https://www.ameland.nl/ vindt ie nog steeds geen Siteimprove. En volgens mij klopt dat.

rudiedirkx commented 7 months ago

Maar we moeten dit wel veel beter testen, want het zou hierdoor ook kunnen dat de pagina heel vaak blijft hangen omdat ie niet in idle0 of idle2 komt omdat er XHR requests blijven gebeuren. Dus gooi er een hoop rare en normale sites doorheen met fetch toegestaan, en kijk wat eruitkomt.

simonbesters commented 7 months ago

Maar we moeten dit wel veel beter testen, want het zou hierdoor ook kunnen dat de pagina heel vaak blijft hangen omdat ie niet in idle0 of idle2 komt omdat er XHR requests blijven gebeuren. Dus gooi er een hoop rare en normale sites doorheen met fetch toegestaan, en kijk wat eruitkomt.

Ik zal morgen een bulk test doen met random en rare sites.

simonbesters commented 7 months ago

Ik heb alle 342 gemeentelijke hoofdwebsites mét en zonder 'fetch' in dip lokaal wappa laten uitvoeren:

fetch vindt veel meer
fetch leek in 1 geval een timeout error geven (maar de site bleek down te zijn https://www.echt-susteren.nl)
de cookie test van Siteimprove!! zat hier ook in, kun je negeren.

Ik denk dat we dit prima kunnen doen en wel de timeouts blijven monitoren in dip.

tabel

rudiedirkx commented 7 months ago

fetch wordt nog niet gebruikt als xhr pattern bron. Dat voegt in dit geval misschien niks toe, maar wel netjes.

    if (request.resourceType() === 'xhr') {

Die moet ook fetch accepteren en daar de xhr patterns op los laten.

rudiedirkx commented 7 months ago

En dan moeten we xhr ook toelaten waar je nu fetch toevoegt.

rudiedirkx commented 7 months ago

En met testen bedoelde ik vooral de sites die niet geoptimaliseerd en getest en veelgebruikt en wettelijk netjes zijn, dus de intranetten en SSO redirects en youtube kanalen etc.

simonbesters commented 7 months ago

En met testen bedoelde ik vooral de sites die niet geoptimaliseerd en getest en veelgebruikt en wettelijk netjes zijn, dus de intranetten en SSO redirects en youtube kanalen etc.

Ik heb ook random ruk websites gedaan. Alleen geen vergelijk zoals bij alle gemeente hoofdwebsites.

simonbesters commented 7 months ago

Die moet ook fetch accepteren en daar de xhr patterns op los laten.

En dan moeten we xhr ook toelaten waar je nu fetch toevoegt.

Toegevoegd. Ik heb me beperkt tot die twee regels qua leesbaarheid verbeteren. Jij hebt waarschijnlijk daar voor heel wappa al ideeën over.

rudiedirkx commented 7 months ago

Ik heb me beperkt tot die twee regels qua leesbaarheid verbeteren.

Ik zou het juist op 1 regel zetten ipv 5 superkorte:

if (['xhr', 'fetch'].includes(request.resourceType())) {

is leesbaarder dan

if ([
'xhr',
'fetch'
].includes(request.resourceType())
) {

Die komma aan het eind van de diff mag je laten staan. Wellicht is je IDE te enthousiast.

rudiedirkx commented 7 months ago

Er gebeuren nog meer rare dingen. Zie diff. Je moet wel je diff scrutinizen voordat je commit he. Niet alleen handig om debugs eruit te halen 😆

rudiedirkx commented 6 months ago

Ik heb de formatting aangepast. Willen we deze nog beter testen, of vertrouwen we meer meer meer xhr/fetch? Dat ie langzamer wordt is niet erg. Dat ie crasht of dingen mist wel. Had jij jouw lijstje websites getest met alleen die eerste verandering (fetch toelaten) of ook met de tweede (fetch analyseren)? Het lijkt me dat er alleen maar betere resultaten uitkomen, maar misschien winnen daardoor wel ineens andere techs van degene die wij verwachten/willen.

simonbesters commented 6 months ago

Willen we deze nog beter testen, of vertrouwen we meer meer meer xhr/fetch?

Nee, deze kan door. Ik heb getest inclusief analyse fetch. Ook gekke kutwebsites.

dipnl / wappalyzer

allow fetch as request #16