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

Allow multiple Speculation Rules #273

Open
midzer opened this issue Dec 3, 2022 · 6 comments
Open

Allow multiple Speculation Rules #273

midzer opened this issue Dec 3, 2022 · 6 comments

Comments

@midzer
Copy link

midzer commented Dec 3, 2022

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

if (isSpecRulesExists()) {

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

Have a nice weekend
midzer

@philwolstenholme
Copy link

philwolstenholme commented Dec 7, 2022

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

@tunetheweb
Copy link
Member

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.

@philwolstenholme
Copy link

philwolstenholme commented Dec 7, 2022

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:

    quicklink/src/index.mjs

    Lines 239 to 258 in e6565ed

    // prerendering preconditions:
    // 1) whether UA supports spec rules.. If not, fallback to prefetch
    if (!hasSpecRulesSupport()) {
    prefetch (urls);
    return Promise.reject(new Error('This browser does not support the speculation rules API. Falling back to prefetch.'));
    }
    // 2) whether spec rules is already defined (and with this we also covered when we have created spec rules before)
    if (isSpecRulesExists()) {
    return Promise.reject(new Error('Speculation Rules is already defined and cannot be altered.'));
    }
    // 3) whether it's a same origin url,
    for (const url of [].concat(urls)) {
    if (!isSameOrigin(url)) {
    return Promise.reject(new Error('Only same origin URLs are allowed: ' + url));
    }
    toPrerender.add(url);
    }

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
Copy link

philwolstenholme commented Dec 7, 2022

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.

@tunetheweb
Copy link
Member

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.

@JiangWeixian
Copy link

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. 🤔

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

4 participants