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

Support history state that builds in stages #17

Open
domenic opened this issue Feb 3, 2021 · 12 comments
Open

Support history state that builds in stages #17

domenic opened this issue Feb 3, 2021 · 12 comments

Comments

@domenic
Copy link
Collaborator

domenic commented Feb 3, 2021

(Moved from slightlyoff/history_api#18 by @jakearchibald and quoted verbatim.)

Imagine the following steps:

  1. User loads image from filesystem
  2. User selects codec
  3. User changes codec settings
  4. Repeat 3 a few times

If each of these was a history step, it either means storing the image from step 1 in each step, or determining current state from previous entries:

const combinedState = Object.assign(
  {},
  ...appHistory.entries
    .slice(0, appHistory.entries.indexOf(appHistory.currentEntry))
    .map((entry) => entry.state)
);

Storing the image in each step sounds bad for memory, especially if it's serialised in each entry.

Should getting the combinedState be made easier? Does this pattern result in lost data when pages are opened in a new window?

@domenic
Copy link
Collaborator Author

domenic commented Feb 3, 2021

I'm very interested in a solution to this problem, and in general to the question of how structured we should make app history state.

Some ideas:

  • Store the image separately somewhere (e.g., IDB, or some sort of session-IDB variant), and storing a pointer to the image in the history state. That feels kind of inconvenient; it'd be nice if history state could just be where everything goes.
  • Store the image in a format that is easily deduplicated by the browser. I think a Blob would suffice for this, or maybe an ImageBitmap. This relies a bit on browser magic though.
  • Have the app store the image in a particular canonical history entry, and retrieve it from there. Either using a generic pattern like you suggest, or using something more app-specific. (E.g. appHistory.entries.find(e => e.key === imageKey).)

Should getting the combinedState be made easier? Does this pattern result in lost data when pages are opened in a new window?

I think you should consider cases like this, similar to cases where the user sends the URL to their friend. If you want the data to be sharable, either on the same computer or across multiple computers, it should probably be in the URL. App history state (like history.state) works best as a session-specific cache for information that can be reconstructed or retrieved from the URL or other persistent information (such as a cookie).

I'm not confident on my thinking here, so pushback welcome.

/cc @tbondwilkinson

@jakearchibald
Copy link

Store the image separately somewhere (e.g., IDB, or some sort of session-IDB variant), and storing a pointer to the image in the history state. That feels kind of inconvenient; it'd be nice if history state could just be where everything goes.

Yeah, thinking of squoosh.app this would complicate our privacy story a lot.

Have the app store the image in a particular canonical history entry, and retrieve it from there. Either using a generic pattern like you suggest, or using something more app-specific.

As a developer, I'd probably come up with some system where some history states are 'keyframes' and others build on that. It's probably best to leave this in developer space and see what patterns come up.

I think you should consider cases like this, similar to cases where the user sends the URL to their friend. If you want the data to be sharable, either on the same computer or across multiple computers, it should probably be in the URL.

Maybe I'm over-indexing on "what if this was a better session storage??".

@tbondwilkinson
Copy link
Contributor

I do think that it would be interesting to think about different ways of serializing the "state" Object to better optimize for use cases like this. But I'm not 100% sure I'm sold on this use case.

My instinct as a developer would be to store the Image somewhere else - e.g., memory, rather than history. With things like bfcache, we don't pay nearly as high a price for keeping things in memory these days, and if we do a full refresh, it's probably okay to hit the network cache to reload that image to put it back into JS memory.

@jakearchibald
Copy link

One reason I'd like to avoid memory is history state persists even if the tab discards.

@tbondwilkinson
Copy link
Contributor

Right, but sometimes things (especially big things) should unload when the tab discards. I don't know if a user would love to see that Chrome is using a ton of RAM storing things in history that are best re-loaded when the tab is loaded back.

@jakearchibald
Copy link

RAM? I figured it wouldn't be RAM, otherwise how would it survive a browser restart?

@tbondwilkinson
Copy link
Contributor

Good point :) I haven't the foggiest how this works under the hood.

I do know that browsers today have limits on what you can store in things like session storage, history etc. (it's been documented as 5MB but it varies). If you want to start to talk about storing large things like images into history, I think that would require a rethink, and I wonder whether that's really a valid use case. What benefit does the user get for having images downloaded locally to their device rather than depending on the network cache or a server round-trip for cases like browser restart? Do they save some ms for that? Is that worth storing many MB of information on my device per website? Would we need a TTL for such resources?

I just worry that making it TOO useful of a application cache means we also would have to make it far more complicated.

@jakearchibald
Copy link

The image may have come from disk rather than the network, so it's not easy to simply reload.

What you're proposing (ram only storage) is a pretty big regression on the current history API. There isn't a lot good about the currently history API, but we probably shouldn't lose what little good there is 😄

@tbondwilkinson
Copy link
Contributor

Oh no, I'm not suggesting the new API should be RAM only. I don't know any of the implementation details.

All I'm saying is, maybe it's okay if the user's upload image has to be uploaded again if the browser crashes.

@domenic
Copy link
Collaborator Author

domenic commented Feb 12, 2021

Note that #36 pushes us toward avoiding storing images or the like in app history state; instead you'd store some sort of cache key into IndexedDB.

I do believe some of the limits on history.state size today are because the browser does keep things in RAM. It also serializes them to disk, but I believe they stay in RAM so that navigation can be relatively faster. @natechapin would know for sure.

@SetTrend
Copy link

SetTrend commented Feb 13, 2021

@domenic kindly pointed me to this thread.

I'm not sure if my concerns truely match this thread's topic, yet let me explain here:

My concern is how a state's members are stored, serialized and deserialized.

Imagine a state like this:

function MyClass(id) { this.id = id; }

const myState = { a: new MyClass(1), b: "x" };

appHistory.push({ myState });

What if the programmer then wrote:

myState.a.id = 2;

appHistory.push({ myState });

... and then navigated back in history, using 'appHistory.back()'.

What would be the resulting value of 'appHistory.current.state.a.id'?

My concern, stated here boils down to:

  • Should there be an automatic deep clone operation involved with appHistory.update/push({ state });?
  • How can a JavaScript be prevented from altering a previous entry's state object's members?

@tbondwilkinson
Copy link
Contributor

I agree that feels like a separate issue, and brings up what the best serialization is for state Objects.

Currently, the history.state is a deep clone. So mutating with myState.a.id would update the next entry, but not the previous. I think we'd probably want to preserve that behavior

But maybe this should be a new issue?

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