This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 29004 - FrozenArray only provides shallow immutability
Summary: FrozenArray only provides shallow immutability
Status: NEW
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: WebIDL (show other bugs)
Version: unspecified
Hardware: All All
: P2 normal
Target Milestone: ---
Assignee: Cameron McCormack
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-29 16:04 UTC by John Mellor
Modified: 2015-08-03 17:37 UTC (History)
10 users (show)

See Also:


Attachments

Description John Mellor 2015-07-29 16:04:48 UTC
Bug 23682 introduced FrozenArray using Object.freeze as a "fixed length array of unmodifiable values" (https://heycam.github.io/webidl/#idl-frozen-array).

But Object.freeze only provides shallow immutability: the values are not writeable, but if the values are objects they can still be mutated.

It might be useful to have a deeper concept of immutability in WebIDL, e.g.:

A) Perhaps FrozenArray should non-recursively call Object.freeze on each of its non-primitive values?

B) Or perhaps you should be able to declare:
  readonly attribute frozen Array<frozen Foo> foos;

For example, in https://github.com/whatwg/notifications/pull/48 I'm adding action buttons to notifications with the following WebIDL:

partial dictionary NotificationOptions {
  sequence<NotificationAction> actions = [];
};
dictionary NotificationAction {
  required DOMString action;
  required DOMString title;
};

and would like the author to be able to read back the values they set using something like:

partial interface Notification {
  readonly attribute FrozenArray<NotificationAction> actions;
};

But currently authors would be able to mutate the NotificationActions (or specifically, the JS objects that WebIDL converts dictionaries to), and this may lead to confusion, for example authors might incorrectly expect that mutating the titles would update the button strings of currently showing or soon to be shown notifications. It would also be inconsistent with all the existing attributes on Notification which are immutable.

See also related discussion at https://github.com/w3c/screen-orientation/issues/13
Comment 1 Tim Guan-tin Chien [:timdream] 2015-07-31 03:26:28 UTC
See related use case here on mozApps API
https://bugzilla.mozilla.org/show_bug.cgi?id=1184439

And "deep freeze" feature implemented in Gecko
https://bugzilla.mozilla.org/show_bug.cgi?id=1186213

And discussion around these use cases in mozilla.dev.platform

https://groups.google.com/d/msg/mozilla.dev.platform/tF6wUXFqj9c/E7YDu2oSWhcJ

It would be nice if the keyword is baked into WebIDL and implemented at this level.
Comment 2 Travis Leithead [MSFT] 2015-07-31 18:05:56 UTC
I want to be cautious here--I agree with what hixie has said in the past, that supplying frozen objects from the platform is an anti-pattern that seems contrary to the platform's "configurable" nature by default. This comment was made on the initial definition of FrozenArray<T> and now you are proposing making this a *deep* operation.

Since we don't generally do this for dictionaries (make them frozen), I'm wondering why we have this need for deep frozen arrays?
Comment 3 Boris Zbarsky 2015-07-31 18:12:21 UTC
Dictionaries are returned by value, not by reference, so there is no need to freeze them when returning them normally.

The corresponding situation for an array is a sequence return value, which likewise never needs to be frozen.

The frozen thing is for when you want to keep returning the same object each time but don't want the caller to be able to modify it.  We can already do that with some IDL interface which has only getters and no setters; the problem is that such things don't play along well with other things people want to do with their arrays (concat, map, filter, etc).  We _could_ enable IDL interfaces that do all that more sanely, or we could just use actual Array objects... the FrozenArray approach is the latter.
Comment 4 Travis Leithead [MSFT] 2015-07-31 18:21:57 UTC
(In reply to Boris Zbarsky from comment #3)
> The frozen thing is for when you want to keep returning the same object each
> time but don't want the caller to be able to modify it.

I think this is where my question is. There's nothing that prevents the platform from keeping a reference to a non-frozen JS array and returning the same instance over and over. Is there another requirement that I'm missing? Such as, are platforms expecting to be able to grow/shrink/change the values in the array instance themselves, while not allowing author code this privilege?
Comment 5 Boris Zbarsky 2015-07-31 18:26:52 UTC
Since the array is frozen, the platform can't grow/shrink/change it either; if the list changes it needs to start returning a new object.  So it's not that the platform is doing anything author code can't do (apart from returning the object to start with).

The idea of freezing is to not have author code accidentally change the array, leading to other author code getting bogus values.  For some APIs, where there is probably only one consumer on the page for the object in question, maybe this doesn't matter.  For other APIs, e.g. anything on Window or Document or Navigator or any other per-page singleton, it's easy to end up with multiple unrelated scripts using the same array.

Note that nothing prevents a spec from defining an API which returns an unfrozen array.  You use the "object" return value and define all your behavior in prose.  The point of FrozenArray is to nudge people towards one particular, safe-by-default, pattern, but if someone feels a strong need to do something different they always can.
Comment 6 Travis Leithead [MSFT] 2015-07-31 19:14:35 UTC
To me, the "frozen" aspect still seems like a completely unnecessary protection. The platform has hitherto only offered very few places where we choose to lock-out modification (e.g., [Unforgeable]). For everything else, if any one author script decides to ```delete Document.prototype.getElementById;``` it affects all author script. The reason this isn't a massive problem today is because author scripts are generally benevolent and don't accidentally make these changes.

I can see the argument that existing array-like interfaces don't, in fact, allow mutation today. Take for example, the plugins collection:

```[].push.call(navigator.plugins, { travis: "test"})```
```[].pop.call(navigator.plugins)```

both of which throw an exception today. These would be the kind of mutations that one author script could do that would damage/inhibit other author script from having a good experience. (Similar to my delete example above.)

However, the following are OK, and do not throw:

```navigator.plugins["100"] = { travis: "test" };```
```navigator.plugins.test = "value";```

which seem valuable for scenarios that want to supplement the array objects or provide additional behaviors.

So, FrozenArray keeps the immutability constraint as currently available in the first set of examples, but further restricts the author's options given what is possible today as in the second examples.

My argument is that we should be not be swinging toward more restrictive, rather we should be swinging toward less restrictive and more flexible. E.g., allowing the plugins array to be supplemented by a couple new entries could be very useful for a compatibility shim, and in that case you *do* want the change to effect all author script.

Furthermore, if the issue is a desire to convert existing array-like things into FrozenArray (like the plugins collection above), it's very back-compat-friendly to go from more restrictive to less restrictive because no existing [successful] author code will already be mutating those arrays. However, there is a chance that author code may already be supplementing existing array-like things in the platform, and a FrozenArray conversion would cause that code to start throwing.
Comment 7 Boris Zbarsky 2015-07-31 19:32:26 UTC
It's really hard to accidentally delete Document.prototype.getElementById.  It's easy to accidentally foo.bar.sort() or pass foo.bar to some library function that modifies it.  Again, the frozenness is meant to prevent accidental modification, not attacks.  And it's just a lot easier to accidentally modify an array you're working with in JS than it is to accidentally modify a random DOM prototype.  That's the whole point of this discussion, and I think arguing that scripts rarely do the latter (though some do, and stuff breaks, in fact) doesn't shed much light on how likely scripts are to do the former.

> These would be the kind of mutations that one author script could do that would
> damage/inhibit other author script from having a good experience. 

Sure.

> (Similar to my delete example above.)

I think it's a lot more common to process an array via a while-loop-with-pop than it is to delete things off standard prototypes, for what it's worth.

> However, the following are OK, and do not throw:
>```navigator.plugins["100"] = { travis: "test" };```

This throws in strict mode per webidl spec and in at least Firefox.  In non-strict mode it's a silent no-op.  The same is true for a frozen array, of course.

>```navigator.plugins.test = "value";```

Note that this would also not throw outside strict mode with a frozen array.  But neither would it work, of course.

So maybe instead of freezing the array, we could mark all the properties 0-length and "length" itself readonly noncofigurable, but allow adding other random properties...  It's a lot more complicated than just freezing, but is the minimal thing with the desired behavior.

> My argument is that we should be not be swinging toward more restrictive,
> rather we should be swinging toward less restrictive and more flexible.

I understand that argument, and maybe we should try to find some middle ground between just letting anyone stomp on the array accidentally via sort() and locking it down completely.

> Furthermore, if the issue is a desire to convert existing array-like things

To some extent, but chances are that's not very web-compatible because of differences in concat() behavior, so this is mostly about new things, I think...
Comment 8 Travis Leithead [MSFT] 2015-07-31 19:47:01 UTC
(In reply to Boris Zbarsky from comment #7)
> So maybe instead of freezing the array, we could mark all the properties
> 0-length and "length" itself readonly noncofigurable, but allow adding other
> random properties...  It's a lot more complicated than just freezing, but is
> the minimal thing with the desired behavior.

The effect that this has seems reasonable to me. It doesn't allow more or less than we have today. I'd like to explore this further.

> > My argument is that we should be not be swinging toward more restrictive,
> > rather we should be swinging toward less restrictive and more flexible.
> 
> I understand that argument, and maybe we should try to find some middle
> ground between just letting anyone stomp on the array accidentally via
> sort() and locking it down completely.

Agree.
Comment 9 Jonas Sicking (Not reading bugmail) 2015-07-31 20:05:23 UTC
One of the reasons that I want to return frozen Arrays from some APIs is that that enables us to return the same object from multiple calls.

For example, it means that the HTMLInputElement.files can return the same object every time that you get it. If the user attaches another file, we simply create a new Array and return that.

If we don't freeze the Array, that would mean that custom properties set on myInputElement.files will appear to "disappear" if a file is added.

Ideally we would use something like "value objects" rather than frozen Arrays, however JS doesn't currently have those so that's not an option.


So I think frozen semantics is what we're actually looking for. Not just "can't add or remove entries to the Array".
Comment 10 Domenic Denicola 2015-07-31 20:13:13 UTC
> So maybe instead of freezing the array, we could mark all the properties 0-length and "length" itself readonly noncofigurable, but allow adding other random properties...  It's a lot more complicated than just freezing, but is the minimal thing with the desired behavior.

Not sure what mark all the properties 0-length means. But in general schemes like this do not work well because there's no way to prevent `array[1000] = "test"` (which, in a real array, will then update `array.length` to be `1001`). Additionally, Jonas's

> If we don't freeze the Array, that would mean that custom properties set on myInputElement.files will appear to "disappear" if a file is added.

is very important to keep in mind.

---

In general I agree that asking for more frozen objects is not great. However I see no problem with objects that have non-writable data properties. (I'd leave them configurable though.)

I don't quite understand how `readonly attribute FrozenArray<NotificationAction> actions` works. Since NotificationAction is a dictionary, and thus reified by value, I assume a new JS object is generated for each array element, each time `.actions` is called. But `.actions` is a frozen array, so its elements cannot change. Maybe there is an implicit "only reify the dictionary once" going on? But it is not obvious to me how that falls out.
Comment 11 Boris Zbarsky 2015-07-31 20:27:05 UTC
> Not sure what mark all the properties 0-length means.

  for (let i = 0; i < foo.length; ++i) {
    Object.defineProperty(foo, i, { ...., configurable: false });
  }

> schemes like this do not work well because there's no way to prevent `array[1000]`

Sure there is.  If "array" is an actual Array or subclass thereof, marking the "length" property as readonly nonconfigurable exactly prevents that, because http://www.ecma-international.org/ecma-262/6.0/#sec-array-exotic-objects-defineownproperty-p-desc step 3.f will cause the property definition to fail.  Simple testcase:

  var arr = []; 
  Object.defineProperty(arr, "length", 
                        { value: 0, writable: false, configurable: false });
  arr[5] = 7;
  console.log(arr[5]) // undefined
  (function() { "use strict"; arr[5] = 7 })(); // throws

Jonas's point is a lot more important, though.  I hadn't though of that problem. :(

> I don't quite understand how `readonly attribute
FrozenArray<NotificationAction> actions` works

A FrozenArray is a JS object (just like "object"), even in terms of its "IDL value".  So in this case, whatever has that "actions" attribute internally stores a JS Array object which is frozen.  The getter returns this internally stored JS Array object.

> Since NotificationAction is a dictionary, and thus reified by value

It's reified by value at the point when the FrozenArray is created.  But after that the FrozenArray just contains the JS object produced from the dictionary.

> Maybe there is an implicit "only reify the dictionary once" going on?

It's not implicit at all.  It's explicit in the creation of the FrozenArray object, which is defined in http://heycam.github.io/webidl/#dfn-create-frozen-array
Comment 12 Domenic Denicola 2015-07-31 20:43:42 UTC
> for (let i = 0; i < foo.length; ++i) {
  
Ah, the "-" was "through", not a hyphen ("zero-length").

> Sure there is.

Huh! Thank you for correcting me.

> It's not implicit at all.  It's explicit in the creation of the FrozenArray object

Thanks, I had not re-read that section enough times.
Comment 13 Travis Leithead [MSFT] 2015-08-03 17:28:29 UTC
(In reply to Jonas Sicking from comment #9)
> For example, it means that the HTMLInputElement.files can return the same
> object every time that you get it. If the user attaches another file, we
> simply create a new Array and return that.
> 
> If we don't freeze the Array, that would mean that custom properties set on
> myInputElement.files will appear to "disappear" if a file is added.
> 
> Ideally we would use something like "value objects" rather than frozen
> Arrays, however JS doesn't currently have those so that's not an option.
> 
> So I think frozen semantics is what we're actually looking for. Not just
> "can't add or remove entries to the Array".

This sounds like you want a pattern for describing (in general) how "live lists" work, except that you're willing to violate the invariant of object-identity from time-to-time.

```var fileListRef = input.files;```

Most of the time (until something causes the list to change):

```fileListRef === input.files;````

I'm concerned that this third-state is a dangerous metaphor because of its unpredictability for the author. With static lists (e.g., result of qSA) the author can understand that the list is always a snapshot, and with live lists (e.g., node.childNodes) the author can understand that the list is always current and up-to-date. Applying FrozenArray to live-list-like scenarios creates a third state whereby you can't make either of these assumptions--and it requires you to have some additional mechanism of being informed of when your list-of-origin changes.

I would much rather have a solution that solves and preserve the invariants of the live list, because I think that is what you actually want for these cases. It's a relatively-slow-updating live list--you just want it to act more array-like than our current flavor of interfaces (HTMLCollection, NodeList, etc.). I think we also want a single 'type' of list rather than having to re-invent another XThingList that has the same semantics.

Using FrozenArray in place of a live list solution will take away:
1. Object identity consistency
2. Object extensibility
which I think are two important characteristics to preserve.

What Boris proposed is close to a live-list replacement: it preserves identity, it preserves extensibility, it forbids mutation of the list values. I think we are close to having something feasible for a live-list replacement. It would meet the requirements of being able to return the same object each time--and rather than replace it with a new one when the list changes, you would simply clear the old list's contents and add the new values (or do something more intelligent to modify the original object's contents). This is possible so long as you don't take the extra step of marking all the [0 to length] properties non-configurable.

Of course, leaving the indexed properties configurable, means that author code could lock them down by setting configurable=false. We'd have to define how platform code would respond to such an event, or suggest that ECMAScript provide a mechanism to prevent properties from become configurable=false in the first place as well as preventing preventExtensions() on the object. (e.g., something like enforceExtensions()).
Comment 14 Boris Zbarsky 2015-08-03 17:33:44 UTC
Travis, just to make sure, you've read all the discussion in bug 23682, yes?  Because you're reiterating some bits that were already discussed to death...
Comment 15 Adam Klein 2015-08-03 17:35:26 UTC
(In reply to Jonas Sicking from comment #9)
> If we don't freeze the Array, that would mean that custom properties set on
> myInputElement.files will appear to "disappear" if a file is added.

I'd like to better understand how this is supposed to work. Which of the following two things are you suggesting should happen when a file is added?

1) The implementation mutates the otherwise-frozen array, extending the "length" and adding a new indexed property.
2) The implementation returns a new frozen array, containing all the elements of the old array plus an additional element.

(1) violates invariants of the language, as non-writable, non-configurable properties should never change.

(2), while avoiding the problem of disappearing "expando" properties (by disallowing them to begin with), still runs into identity issues. If myInputElement.files has been stored somewhere, the old copy won't be updated. It will also fail === comparisons with the new version. Similar identity-based problems arise with using it as a key in a Map/Set/WeakMap/WeakSet.
Comment 16 Boris Zbarsky 2015-08-03 17:37:27 UTC
> Which of the following two things are you suggesting should happen when a file
> is added?

#2.  Again, please read bug 23682.