quicklink icon indicating copy to clipboard operation
quicklink copied to clipboard

Allow multiple Speculation Rules

Open midzer opened this issue 2 years ago • 6 comments

Is your feature request related to a problem? Please describe. According to https://developer.chrome.com/blog/prerender-pages/#multiple-speculation-rules the Speculation API allows multiple Speculation Rules. Right now prerender: true can only prerender a single URL in viewport.

Describe the solution you'd like Remove https://github.com/GoogleChromeLabs/quicklink/blob/e6565ed12b4872b67a862b9fe2198fe0d73cb485/src/index.mjs#L247

Additional context Would be happy to open a PR for this.

Have a nice weekend midzer

midzer avatar Dec 03 '22 04:12 midzer

I noticed this today too, did earlier versions of the API only support one script element, perhaps?

test/test-prerender-only.html can be used to reproduce the issue. Note how in the screenshot below, only 1.html will be prerendered:

A screenshot showing the test-prerender-only.html HTML file from this repo running in a browser. A speculation rule set has only been created for one of the multiple links on the page

philwolstenholme avatar Dec 07 '22 19:12 philwolstenholme

did earlier versions of the API only support one script element, perhaps?

Yes that's correct. It's a relatively recent change (Chrome 107 I think) that allowed multiple prerenders.

tunetheweb avatar Dec 07 '22 19:12 tunetheweb

I've had a look around the repo and made a few changes locally. I think there are a few things we will need to do:

  • Remove isSpecRulesExists() function definition and usage
  • Remove prerenderLimit const definition
  • Remove if (toPrerender.size < prerenderLimit) { check
  • Add toPrerender.clear(); to the 'reset' function returned from listen() (toPrefetch.clear(); is there already)
  • Call toPrerender.clear(); after adding the Speculation Rules script elements to prevent a build up of the same URLs being included as new links are found on scroll (see example below), maybe add a mechanism to avoid adding a URL twice if a script element is created for it once, then again when the user scrolls back to that part of the page?
  • Remove or update comments prerendering conditions comments now that '2)' has been removed: https://github.com/GoogleChromeLabs/quicklink/blob/e6565ed12b4872b67a862b9fe2198fe0d73cb485/src/index.mjs#L239-L258

Example of not clearing out the toPrerender set:

<script type="speculationrules">
  { "prerender": [{ "source": "list", "urls": ["http://localhost:8080/work"] }] }
</script>
<script type="speculationrules">
  { "prerender": [{ "source": "list", "urls": ["http://localhost:8080/work", "http://localhost:8080/contact"] }] }
</script>
<script type="speculationrules">
  { "prerender": [{ "source": "list", "urls": ["http://localhost:8080/work", "http://localhost:8080/contact", "http://localhost:8080/github-stars"] }] }
</script>
<script type="speculationrules">
  { "prerender": [{ "source": "list", "urls": ["http://localhost:8080/work", "http://localhost:8080/contact", "http://localhost:8080/github-stars", "http://localhost:8080/github-stars?no-js"] }] }
</script>

philwolstenholme avatar Dec 07 '22 20:12 philwolstenholme

All that said, I did quite quickly start seeing Max number of prerendering exceeded/MaxNumOfRunningPrerendersExceeded messages in the experimental Prerender2 dev tools panel, so I'm thinking that Quicklink's approach of prerendering anything in the viewport might be a bit too resource intensive compared to only prerendering links that are being hovered over or touchstart'ed on.

philwolstenholme avatar Dec 07 '22 21:12 philwolstenholme

Yeah there's current a limit of 10 prerenders. IMHO that is more than enough, and prerendering even that many (which I could easily see happening if prerendering all in-viewport links) seems like a lot to me.

tunetheweb avatar Dec 07 '22 21:12 tunetheweb

quicklink export prerender API, currently, if call prerender multiple times, still throw this error. If user use prerender API manual, I think we should not limit prerender urls size.

Maybe should mv isSpecRulesExists check function into listen. 🤔

JiangWeixian avatar Feb 02 '23 08:02 JiangWeixian