webidl icon indicating copy to clipboard operation
webidl copied to clipboard

FrozenArray<SomeDictionary> seems like a bad idea.

Open jan-ivar opened this issue 3 years ago • 7 comments

Some reasons:

  • Violates § 6.1. Attributes should behave like data properties because dictionaries have copy semantics
  • Copies are mutable https://github.com/w3c/webrtc-extensions/issues/136
  • creates a loophole for returning mutable dictionaries as attributes https://github.com/w3c/webrtc-extensions/issues/137
  • crbug 946659 exacerbates this anti-pattern https://github.com/w3c/webrtc-extensions/issues/135

ObservableArray<T> says: "The parameterized type T must not be a dictionary type, sequence type, record type, or observable array type." Any reason not to say the same about FrozenArray<T>?

jan-ivar avatar Dec 13 '22 15:12 jan-ivar

Likewise it should not permit sequence<T>, record<K, V>, ObservableArray<T>, or (at least not without additional requirements) Promise<T>, nor annotated or nullable types with these inner types. I think this is an oversight.

(Edit: which is the same thing the ObservableArray quote above says, modulo taking into account annotated and nullable variants and Promises. If Promises were permitted, which seems inadvisable, I would expect the requirement to extend to their own parameterized types.)

bathos avatar Dec 13 '22 16:12 bathos

In general I'm not a big fan of FrozenArray<>. It has a highly misleading name and uses a feature of JavaScript that's not exactly embraced at-large. I would not mind avoiding it altogether for new APIs.

annevk avatar Dec 13 '22 16:12 annevk

I think FrozenArray<> has its place. It's the clearly-correct choice for something that cannot ever change after creation. Its name seems very on-point to me.

I agree that you should not be able to put by-value types inside FrozenArrays. I think we should at least copy over the existing statement for ObservableArray, and probably expand it to annotated or nullable types with those as inner types.

I don't understand why we'd restrict promises from being held by these types.

domenic avatar Dec 14 '22 01:12 domenic

I don't understand why we'd restrict promises from being held by these types.

I was thinking that in the absence of a definite use case, the added complexity of specifying the constraints right wouldn’t be worthwhile — shouldn’t FrozenArray<Promise<SomeDictionary>> be invalid just as FrozenArray<SomeDictionary> should be? But it’s true that e.g. FrozenArray<Promise<SomeInterface>> wouldn’t be problematic, it’s just not clear to me when it would have any utility.

bathos avatar Dec 14 '22 02:12 bathos

I think FrozenArray<> has its place. It's the clearly-correct choice for something that cannot ever change after creation. Its name seems very on-point to me.

Where the name misleads perhaps is spec-authors assuming entries become constants, not realizing create a frozen array is shallow or that there's no way to "freeze" an interface. crbug 946659 doesn't help (as in: WebIDL will freeze everything for me wholesale, which is not the case). Disallowing dictionaries should help (one less nestable type to be confused over).

The name might be fine if we could clarify a bit where it says "A frozen array type is a parameterized type whose values are references to objects that hold a fixed length array of unmodifiable values."

I worry readers might assume the interfaces in e.g. FrozenArray<PerformanceServerTiming> are "unmodifiable" for reasons other than PerformanceServerTiming's own attributes all being readonly.

The only thing to dispel this seems to be: "There is no way to represent a constant frozen array value in IDL." — which reads like a WebIDL bug more than any guiding design choice (are non-constant interfaces 👍 👎?)

shouldn’t FrozenArray<Promise<SomeDictionary>> be invalid just as FrozenArray<SomeDictionary> should be?

Semantically perhaps, to avoid another loophole. Technically though, the former doesn't seem to violate any design rule AFAICT. In some sense, a Promise is just another interface (used to be), with no requirement to act one way or another.

jan-ivar avatar Dec 14 '22 16:12 jan-ivar

In some sense, a Promise is just another interface (used to be), with no requirement to act one way or another.

The Web IDL Promise<T> type does have an interface-like mapping to a specific ES Promise object by identity, yes. It also associates a conversion algorithm with that Promise’s eventual settled value (which can also lead to firing unhandledrejection events). Since the conversion happens only zero-or-one times, it’s sound in a letter-of-the-law sense. On the other hand this indirection-by-promise-wrapper amounts to a slightly worse version (in a potential-for-side-effects sense) of what you’d get with directly “memoized” conversion for FrozenArray<SomeDictionary>. My sense was that neither is desirable, but there may be good reasons to want it that I’m just not thinking of.

(AFAICT we agree — just trying to articulate why it seems fraught to me.)

bathos avatar Dec 14 '22 16:12 bathos

The name might be fine if we could clarify a bit where it says "A frozen array type is a parameterized type whose values are references to objects that hold a fixed length array of unmodifiable values."

Agreed, that sentence seems actively misleading.

The only thing to dispel this seems to be: "There is no way to represent a constant frozen array value in IDL." — which reads like a WebIDL bug more than any guiding design choice

That's just due to the fact that Web IDL's constant grammar is very limited: you can do basic stuff like true, false, 1.0, and [], but nothing else.

(are non-constant interfaces +1 -1?)

You mean, interfaces with non-readonly properties? Those are totally fine!

domenic avatar Dec 15 '22 01:12 domenic