App history state (im)mutability

[domenic]

Continuing from #7 (comment), where @tbondwilkinson notes a sharp edge of the existing history.state API:

history.pushState({ test: 2 }, null, '');
console.log(history.state.test); // 2
history.state.test = 3;
console.log(history.state.test); // 3

// refresh the page

console.log(history.state.test); // 2

That is, because it allows any arbitrary JavaScript object, it allows mutable JavaScript objects. But the object is only serialized into (semi-)persistent storage when you call history.pushState() or history.replaceState().

The current proposal duplicates this sharp edge with app history state. E.g.,

appHistory.push({ state: { test: 2 } });
console.log(appHistory.current.state.test); // 2
appHistory.current.state.test = 3;
console.log(appHistory.current.state.test); // 3

// refresh the page

console.log(appHistory.current.state.test); // 2

This seems quite bad to me, and I think we should consider a way to fix it, if we can do so without interfering with other use cases. I'll post a reply with some suggestions.

[domenic]

Things that don't work

Note that we can't use Object.freeze() (or some deep-freeze variant) as a way of locking down passed in objects, because many objects store state in ways other than object properties. E.g. Set instances

const frozenSet = Object.freeze(new Set());

// Still works
frozenSet.add(2);

Only allow immutable primitives

Only allow immutable primitives (numbers, strings, booleans, bigints, symbols, etc.). No objects allowed.

We could make this slightly less painful by having the entry.state property be a maplike which lets you set keys and values. So e.g. you might do the above using something like

// Auto-convert object literals to entries in the app history state map
appHistory.push({ state: { test: 2 } });

console.log(appHistory.current.state.get('test')); // 2
appHistory.current.state.set('test', 2);
console.log(appHistory.current.state.get('test')); // 3

// refresh the page

console.log(appHistory.current.state.get('test')); // 3

In the future, if records and tuples ever happen, they would also be allowed, which would give you the ability to contain larger object graphs.

I suspect this would be fairly constraining for web developers, and would push them toward using app history state to store cache keys or similar which then have to be looked up in out of band sources like sessionStorage or IndexedDB. I'm not sure that's a great idea, but it would at least make it obvious to them what's going on...

Try to more aggressively snapshot the state?

We could imagine trying to serialize the current value of state on document unload. The main downfall here is that this might not always succeed, e.g.

appHistory.push({ state: { test: 2 } });
console.log(appHistory.current.state.test); // 2
appHistory.current.state.test = 3;
console.log(appHistory.current.state.test); // 3

// refresh the page: browser snapshots current value of state.

console.log(appHistory.current.state.test); // 3, it works!

appHistory.current.state.test = document.body;

// refresh the page: browser tries to snapshot current value of state,
// but fails because you can't serialize a DOM node

console.log(appHistory.current.state.test); // ??? probably throws an exception because appHistory.current.state is null?

[Yay295]

Instead of having state be a property it could instead be a getter (as in entry.getState()) that returns a copy of the state. This would allow the state to still contain arbitrary objects but makes it more clear that changing it doesn't actually change what's stored in the history entry.

I suppose there could also be a setState() function, but I don't think that's as necessary since .update({ state }) exists.

Having the state property defined to return a copy could also be an option, but it's not as implicitly clear.

[tbondwilkinson]

I do think:

appHistory.getCurrent().state

or

appHsitory.current.getState()

are compelling minor changes to communicate to users that the state Object cannot be mutated in place, and that changes to it will not affect other calls to appHistory.current

[domenic]

Just changing it to a function could definitely help. However, that would work best if we re-cloned the data upon each access, i.e.:

Return a new clone on every access

const stateInput = { test: 2 };
appHistory.push({ state: stateInput });

const state1 = appHistory.current.state();
console.assert(stateInput !== state1);

const state2 = appHistory.current.state();
console.assert(state1 !== state2);

This kind of behavior makes the situations from the OP very clear then:

appHistory.push({ state: { test: 2 } });
console.log(appHistory.current.state().test); // 2
appHistory.current.state().test = 3;          // modifies a throwaway object
console.log(appHistory.current.state().test); // still 2

// refresh the page

console.log(appHistory.current.state().test); // 2

I wonder if this kind of cloning if that would be too expensive, or would be OK? It kind of depends on how large the state is that people store here, and how often they access it, which I don't have a good intuition for.

[frehner]

Would it be a deep clone or a shallow clone under this proposal?

[domenic]

Deep clone, in particular a structured clone

[tbondwilkinson]

I actually want to push back on this:

Note that we can't use Object.freeze() (or some deep-freeze variant) as a way of locking down passed in objects, because many objects store state in ways other than object properties. E.g. Set instances

In today's history API, history.state does serialization to raw Objects, like JSON (but not JSON), that means that you cannot have an Object with function properties:

class Foo {
  bar() {}
}

history.pushState(new Foo(), '');
history.state // {}

So I think we could have a deeply immutable state Object without having to worry about mutability with something like methods.

So I think my preference would definitely be a state Object that gets serialized without methods or prototypes, basically just supporting Object literals, arrays, and primitives.

[tbondwilkinson]

And another note is that history.pushState actually errors of the properties of an Object cannot be cloned, e.g.:

// Uncaught DOMException: Failed to execute 'pushState' on 'History': foo() {} could not be cloned.
// at <anonymous>:1:9
history.pushState({foo() {}}, '');

[domenic]

I actually want to push back on this:

The code examples you show exemplify how the current history API snapshots the passed-in value. (Using structured clone.) They don't show the passed-in value being immutable, though. It's making object graphs immutable which is not really possible in JavaScript; snapshotting them is fine.

[tbondwilkinson]

Can you explain a little more why it's not possible to make the Object graph immutable?

I would argue that if it's serializable by the history API today, it's possible to make it deeply immutable

[domenic]

Yeah, "not possible" is overstating it. But I think the connection between snapshotting and immutability is pretty weak. The only thing they share in common is that they both have to walk the object graph.

What I think you're getting at is something like:

Custom snapshot and semi-deep freeze

• Creates a snapshot of the object graph, like structured clone, but outlaws any objects which can store internal mutable state, like Map, Set, typed arrays, ArrayBuffer, RegExp, and various serializable web platform primitives like ImageData or ImageBitmap
• Then does a pass over the snapshot and semi-deep-freezes it

I say "semi" deep freezes because we wouldn't freeze their prototype chains, because it could contain things like Object.prototype. That would mean you could have code like

Object.prototype.test = 2;
history.pushState({ }, null, '');
console.log(history.state.test); // 2
history.state.test = 3;
console.log(history.state.test); // 3

// refresh the page

console.log(history.state.test); // undefined

but I don't think we're really concerned about that case.

My main concern with such an operation is that it'd be rather un-idiomatic and unprecedented for the web platform. I like "Only allow immutable primitives" or "Return a new clone on every access" a good bit better from that perspective...

[tbondwilkinson]

Well, that example is a bit misleading because the serialized representation itself does not serialize the test property. It's the access of that after the fact that does, because state is deserialized as an Object, and Object happens to have a property on its prototype.

I'm also fine with 'return a new clone on every access' but your point about performance does give me pause.

But for another perspective, we store basically the max allowable size in session storage and do frequent deserializations (we store a string representation of data), so maybe it's not so bad after all. But if we go down this route, we may want to consider some way to segment the data so that if you do end up having a very large data object you want to store in state, you could read a portion of that from the state Object without having to deserialize the entire Object.

But then maybe we can just say "If you have performance issues, you should be using session storage to link to your history key, rather than history state itself"

What does "only allow immutable primitives" mean?

[domenic]

What does "only allow immutable primitives" mean?

I was referrring to the solution proposed in #36 (comment)

[tbondwilkinson]

I agree with your assessment that without language support for immutable Objects or record/tuple that's a pretty hard sell in terms of restrictiveness.

I don't like any of our options much here, but I think the best option is probably structured clone. I'll dream of a day when JavaScript supports immutable types (I imagine, if you passed in immutable Objects into the appHistory APIs, that immutableness would be preserved, so there is some forward compatibility with that direction)

[domenic]

I don't like any of our options much here, but I think the best option is probably structured clone.

To confirm, you mean Return a new clone on every access, not Custom snapshot and semi-deep freeze, right?

[tbondwilkinson]

Correct

[domenic]

With this in place, I think we should add an explicit setState(). appHistory.current.setState(newState) looks better than appHistory.update({ state: newState }).