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

Navigations while a navigation is ongoing: Does async make sense? #22

Closed
SetTrend opened this issue Feb 2, 2021 · 9 comments
Closed

Comments

@SetTrend
Copy link

SetTrend commented Feb 2, 2021

At https://github.com/slightlyoff/history_api/blob/master/app_history.md#navigations-while-a-navigation-is-ongoing it reads:

In this proposal, any interceptable navigations are queued up, one after the other

Does it make sense to have navigation be asynchronous if it's serialized in the end anyway?

Wouldn't it be sufficient to have the implementor of the event handler decide on his/her own whether to utilize promises from within the handler by creating/awaiting promises locally?

@domenic
Copy link
Collaborator

domenic commented Feb 2, 2021

Does it make sense to have navigation be asynchronous if it's serialized in the end anyway?

I don't really understand the question. It's because they're asynchronous, that serializing them is necessary. The alternative, of running them in parallel, seems really bad---it'd just be a race to see who finishes first, or maybe both of them could finish!

Wouldn't it be sufficient to have the implementor of the event handler decide on his/her own whether to utilize promises from within the handler by creating/awaiting promises locally?

I'm not sure what this means. Could you give a code example?

@SetTrend
Copy link
Author

SetTrend commented Feb 2, 2021

I see, let me explain:

I would make calls to appHistory.pushNewEntry/appHistory.updateCurrentEntry() synchronous, i.e. one would be executed after the other.

For the following reasons:

  1. When they are synchronous functions, no queuing effort is necessary. They are called one after the other by default.
  2. In the example Queued up single-page navigations much effort has been taken to serialize the navigation call. But that's an application detail that shouldn't be dealt with by the history API. I believe it to be too obtrusive when the history API bloats on an application's concerns. I would expect the navigate event to be synchronous, expecting to return immediately. A well-behaved application is then supposed to queue whatever it believes needs to be done using its own queueing logic, probably triggered by a Promise launched by the implementer from within his/her navigate handler.

@domenic
Copy link
Collaborator

domenic commented Feb 2, 2021

I see. Your proposal goes against one of the main goals of this proposal, which is to make asynchronous navigation (i.e., fetching the appropriate data from the server, performing potentially-asynchronous component initialization, etc.) a first-class part of the history API and navigation lifecycle. See more discussion in https://github.com/slightlyoff/history_api/blob/master/app_history.md#goals and https://github.com/slightlyoff/history_api/blob/master/app_history.md#measuring-standardized-single-page-navigations

@SetTrend
Copy link
Author

SetTrend commented Feb 2, 2021

I can't find asynchronous as one of the goals in this list.

Wouldn't it suffice to just run respondWith() asynchronously? I'm not sure what's the benefit in having the above calls and the navigate event done asynchronously.

@domenic
Copy link
Collaborator

domenic commented Feb 2, 2021

Did you read https://github.com/slightlyoff/history_api/blob/master/app_history.md#measuring-standardized-single-page-navigations ? In particular, giving the browser---and the rest of the app---an idea of how long the navigation takes, and when one is in progress, is important. Single-page app navs has been crippled so far by being just a single instant in time (whenever history.pushState() is called); a major purpose of this proposal is to fix that, making them like multi-page navs that have a separate start time and end time.

It also appears to be important for accessibility technology; see some discussions in w3c/aria#1353.

@SetTrend
Copy link
Author

SetTrend commented Feb 2, 2021

I'm sorry to bother you, but even after reading these I don't see a need for asynchronous execution.

Perhaps I'm having a blind spot, but from the proposal doc's I don't even see a requirement for the respondWith() function. Why should it be impractical to move all what's represented by respondWith() into navigate itself?

I.e., instead:

appHistory.addEventListener("navigate", e => {
  // Don't intercept cross-origin navigations; let the browser handle those normally.
  if (!e.sameOrigin) {
    return;
  }

  // Don't intercept scroll-to-fragment or scroll-to-text-fragment navigations.
  if (e.fragmentTarget) {
    return;
  }

  if (e.formData) {
    e.respondWith(processFormDataAndUpdateUI(e.formData));
  } else {
    e.respondWith(doSinglePageAppNav(e.destinationEntry));
  }
});

... write ...

appHistory.addEventListener("navigate", e => {
  // Don't intercept cross-origin navigations; let the browser handle those normally.
  if (!e.sameOrigin) {
    return;
  }

  // Don't intercept scroll-to-fragment or scroll-to-text-fragment navigations.
  if (e.fragmentTarget) {
    return;
  }

  if (e.formData) {
    processFormDataAndUpdateUI(e.formData);
  } else {
    doSinglePageAppNav(e.destinationEntry);
  }
});

I guess that would perform the same purpose.

@domenic
Copy link
Collaborator

domenic commented Feb 2, 2021

Ah! What you may be missing is that processFormDataAndUpdateUI and doSinglePageAppNav are async functions that return promises. Those promises could do various things, such as

  • Succeed immediately, e.g. if the navigation is instant
  • Fail immediately, e.g. if trying to navigate to a route you know is bad
  • Fail, after some time, e.g. due to a network failure fetching the relevant data, or the server rejecting a form submission
  • Succeed, after some time, e.g. due to the server taking a while to fetch the data

By passing the promise to respondWith(), the browser gets the information about (1) did the navigation succeed/fail; (2) how long did the navigation take. It also then allows the browser to avoid interrupting such an ongoing navigation with another one, without the app explicitly opting in to such behavior.

Without passing those promises to respondWith(), the browser has no idea of the navigation succeeded or failed. And it has no idea how long it took. And it has no ability to prevent races between multiple navigations, by using a queue.

@domenic
Copy link
Collaborator

domenic commented Feb 3, 2021

I'll close this since I think we've had a good discussion here that's mostly concluded (?), and I'm looking to move issues to the new repo, but please feel free to open a new issue over there if you think this is still unclear. Also, note that the pending navigations feature is receiving some good scrutiny from @esprehn and definitely needs some work: see WICG/navigation-api#10, WICG/navigation-api#11, and WICG/navigation-api#13.

@domenic domenic closed this as completed Feb 3, 2021
@SetTrend
Copy link
Author

SetTrend commented Feb 4, 2021

Sure, please go ahead. Thank you for for massive efforts in this!

I still have a feeling that asynchronous overhead is not required here, particularly since navigate calls are getting serialized in the end. But I will need a day or two to reconsider. Perhaps the next version of the spec will shed more light on the intended behaviour, so I can better see the requirement for a promise.

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

2 participants