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

Early Design Review: document.prerendering #613

Closed
bokand opened this issue Mar 1, 2021 · 7 comments
Closed

Early Design Review: document.prerendering #613

bokand opened this issue Mar 1, 2021 · 7 comments
Assignees
Labels
Resolution: satisfied The TAG is satisfied with this design Review type: CG early review An early review of general direction from a Community Group Venue: WHATWG

Comments

@bokand
Copy link

bokand commented Mar 1, 2021

HIQaH! QaH! TAG!

I'm requesting a TAG review of document.prerendering.

We propose introducing a boolean property document.prerendering and associated change event to distinguish prerendering browsing contexts from regular ones for next-generation prerendering on the web, described more broadly at https://github.com/jeremyroman/alternate-loading-modes.

Further details:

You should also know that...

This is a small piece of a large effort to bring back prerendering in a more predictable and standardized way. The entirety of the prerendering feature is a very large body of work so we thought it be good to separate out parts of it, where it makes sense, for ease of review. This feature would only ship as part of a larger launch of a new prerendering mode.

Also, old versions of prerendering used a special value of the document.visibilityState API for this purpose, but it has since been unshipped and removed from the spec. We've considered whether to bring that back but believe it had some shortcomings (noted in w3c/page-visibility#59) and would be better addressed by an API not tied to visibility.

We'd prefer the TAG provide feedback as:

💬 leave review feedback as a comment in this issue and @-notify @bokand

@bokand bokand added Progress: untriaged Review type: CG early review An early review of general direction from a Community Group labels Mar 1, 2021
@torgo torgo added this to the 2021-03-15-week milestone Mar 10, 2021
@plinss
Copy link
Member

plinss commented Mar 16, 2021

Is the change from prerendering==true to prerendering==false a one-time change? Or can a document go from prerendering==true back to prerendering false?

@plinss
Copy link
Member

plinss commented Mar 16, 2021

I'm also concerned with the prerendering state being a boolean. What's the possibility of there being other variants of prerendering state in the future? An enum would be more extensible.

@plinss
Copy link
Member

plinss commented Mar 16, 2021

I'm also concerned with adding more properties to document. It may be useful to consider making a rendering state (or somesuch) object that collects this and other (future) similar properties.

@bokand
Copy link
Author

bokand commented Mar 29, 2021

Hi, thank you for the responses and sorry for the delay - I've been OOO for the last two weeks.

Is the change from prerendering==true to prerendering==false a one-time change? Or can a document go from prerendering==true back to prerendering false?

Assuming you mean "can a document go from prerendering==false to prerendering==true" - yes, that is a potential case that comes from the portals proposal. In that proposal, on navigation, the initial page can be "adopted" into a portal on the destination page. In that case, we would want to set document.prerendering == true.

I'm also concerned with the prerendering state being a boolean. What's the possibility of there being other variants of prerendering state in the future? An enum would be more extensible.

I'm somewhat weary of using an enum given the experience with document.visibilityState. It seems that in practice, an enum that's semantically a bool will cause authors to write code like:

if (document.state == 'prerendering')
  doPrerender();
else
  doRegular();

Which will break when a new enum is introduced. Given we don't currently anticipate anything like this and, should it come up, seems like it'd be more appropriate as additional information on something like a renderingState object as proposed below, a bool seems preferable to me.

I'm also concerned with adding more properties to document. It may be useful to consider making a rendering state (or somesuch) object that collects this and other (future) similar properties.

That sounds reasonable to me. My only worry is there aren't additional such properties and it ends up being an object with a single property. Would it make sense to move document.visibilityState into that as well (and any other related properties, do we know of more?) and then spec document.visibilityState to be a shorthand for document.renderingState.visibilityState?

I don't know...it seems like a overkill for a such a small addition but agree that most changes are small individually but can add up to a significant mass.

@hober
Copy link
Contributor

hober commented May 11, 2021

Hi @bokand!

I'm requesting a TAG review of document.prerendering.

We propose introducing a boolean property document.prerendering and associated change event to distinguish prerendering browsing contexts from regular ones for next-generation prerendering on the web, described more broadly at https://github.com/jeremyroman/alternate-loading-modes.
[…]
You should also know that...

This is a small piece of a large effort to bring back prerendering in a more predictable and standardized way. The entirety of the prerendering feature is a very large body of work so we thought it be good to separate out parts of it, where it makes sense, for ease of review. This feature would only ship as part of a larger launch of a new prerendering mode.

We really appreciate you breaking off a smaller piece of your work to make reviewing it more manageable. In this case, though, it's a smaller piece of your work that appears to stronly depend on other work of yours being in place too. For instance, the web platform doesn't currently have prerendering browsing contexts, so it seems premature to expose a DOM property that can be used to distinguish them from other types of browsing contexts. Maybe we should reschedule this review for after you've requested reviews on & we've reviewed the other things you're working on that this depends on.

@hober hober added the Progress: propose closing we think it should be closed but are waiting on some feedback or consensus label May 11, 2021
@bokand
Copy link
Author

bokand commented May 11, 2021

Hi,

I was mainly looking for an early gut-check that this is a reasonable alternative to the previously removed visibilityState API (prerendering in some form has and continues to ship in some browsers).

Given there's no obvious objections (modulo @plinss' input) I'm happy to leave review of the details until the fuller feature review. I'm not sure what that means for this issue but feel free to put whatever deferring tags and/or close.

Thanks!

@atanassov
Copy link

Hi @bokand, after another look at the issue during our plenary session we are happy with the overall direction and you taking @plinss feedback on the overall API shape.

We look forward to seeing further progress and are happy to engage again as needed. Thank you for working with us.

@atanassov atanassov added Resolution: satisfied The TAG is satisfied with this design and removed Progress: propose closing we think it should be closed but are waiting on some feedback or consensus labels Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: satisfied The TAG is satisfied with this design Review type: CG early review An early review of general direction from a Community Group Venue: WHATWG
Projects
None yet
Development

No branches or pull requests

5 participants