Re: [heycam/webidl] Specify open dictionaries. (#180)

Recapping here, as although the contentious parts of this design are actually quite small, they directly affect Web-developer-facing APIs and I want to make sure we do the right thing.

### Context

Currently, the platform let’s you specify passing an options bag to operations using plain JS objects, e.g.:

```webidl
interface Foo {
    void doStuff(DoStuffOptions);
};

dictionary DoStuffOptions {
    boolean prop;
    boolean anotherProp;
    DOMString? name;
};
```

This is then used in JS like so:

```js
foo.doStuff({ prop: true, anotherProp: false, name: "blah" });
```

Note that as this essentially calls the [Get abstract operation](https://tc39.github.io/ecma262/#sec-get-o-p) of the option bag, it reaches into the prototype chain. This includes getters and setters on the object’s prototype, the common way for attributes to be exposed on the platform. This enables neat use cases such as using the attributes exposed on a previous request to set the options of a new one:

```js
var r = new Request(url, currentRequest);
// where currentRequest is itself an instance of Request.
```

### The Case for Open Dictonaries

For cases where you don't know beforehand all the possible names dictionary members can take (maybe because this could be application specific, e.g.: URL parameters, or specified in a different layer of the platform, e.g.: HTTP headers), you wan't to be able to specify an open (or semi-open dictionary).

The exact syntax by which this will be done in WebIDL is not super important, but currently, here’s what's considered:

```webidl
interface Foo {
    void doStuff(record<DOMString,DOMString>);
};
```

or:

```webidl
interface Foo {
    void doStuff(dictionary<DOMString,DOMString>);
};
```

The above simply means that you can pass the `.doStuff()` method a plain JS object with strings as keys and strings as values. The WebIDL won't tell you anything about what keys are acceptable though prose around it can. Only strings are supported as key types for now (there’s three types of strings in WebIDL, hence the need to specify the key type), and values could take any types, including unions of types.

### Developer Expectations

**From a web developer’s perspective, the precise syntax of this WebIDL construct has little importance; Web developers encounter such APIs essentially through examples** along the lines of:

```js
bar.doSomething({ k: true, y: false, z: 123 });
```

And so how these APIs are actually specified doesn’t (and shouldn’t) affect Web developers. For them, they all look the same and should thus behave the same.

There’s a catch, here, though, which may lead to subtle bugs in application-level code.

### Difference in Design between Open and Regular Dirctionaries

While properties of the current dictionary type reach into the prototype chain, the proposed design for open dictionaries (the `record<KeyType,ValueType>` or `dictionary<KeyType,ValueType>` syntax suggested above) has a similar design to structured cloning as it relies on [EnumerableOwnProperties](https://tc39.github.io/ecma262/#sec-enumerableownproperties) and therefore **only looks at the properties found on the object itself, not those on the prototype chain**.

So for example, doing something like this wouldn’t yield the expected outcome at all:

```js
var defaultOptions = {
    color: "blue",
    textSize: "16px",
    fontFamily: "Times New Roman"
};

var options = Object.create(defaultOptions);
options.textDecoration = "underline";
CSS.toCSSText(options);
```

which would yield:

```js
"text-decoration: underline;"
```

if specified as:

```webidl
namespace CSS {
  DOMSTring toCSSText(record<DOMString,DOMString> styles);
};
```

but:

```js
"text-decoration: underline; color: blue; text-size: 16px; font-family: Times New Roman"
```
if specified like so:

```webidl
namespace CSS {
  DOMSTring toCSSText(StyleRules styles);
};

dictionary StyleRules {
  DOMString color;
  DOMString textSize;
  DOMString fontFamily;
  DOMString textDecoration;
};
```

### Further Concerns with Half-Open Dictionaries

To make matters worse, the situation would get even more confusing with half-open dictionaries (pseudo syntax—as these are still in the works):

```webidl
namespace CSS {
  DOMSTring toCSSText(StyleRules styles);
};

dictionary<DOMString,DOMString> StyleRules {
  DOMString color;
  DOMString textSize;
};
```

Which in our above configuration would yield (missing `fontFamily`):

```js
"text-decoration: underline; color: blue; text-size: 16px;"
```

### Work-Around

Arguably, the following would give the desired/expected outcome in all above cases:

```js
var options = Object.assign({ textDecoration: "underline" }, defaultOptions);
CSS.toCSSText(options);
```

### Other Problematic Example

But the following wouldn’t:

```js
function DefaultOptions(options) {
    for (k in options)
         this[k] = options[k];
}

DefaultOptions.prototype.fontSize = "16px";
// etc…

var options = new DefaultOptions({ textDecoration: "underline" });
CSS.toCSSText(options);
```

### Different Design Solutions and Their Respective Issues

#### 1. Leave `dictionary` As Is, Add `dictionary<T,T>` which doesn't reach into the Prototype Chain

This is the current design. As described above, its main issue is that it reaches into the prototype chain for specified members but not for unspecified ones. It's main advantage is that it maps the newly clarified "records vs. classes" semantics TC-39 agreed upon as described in @domenic's comment above, see https://github.com/heycam/webidl/pull/180#issuecomment-251669022.

#### 2. Leave `dictionary` As Is, Add `record<T,T>` which doesn't reach into the Prototype Chain

This is a variant of the current design. It has the same issues and benefits as the previous ones, but makes it clear to API designers and implementers that the behavior is different. This doesn't clarify the issue for web developers, though.

#### 3. Leave `dictionary` As Is, Add `dictionary<T,T>` which also reaches into the Prototype Chain

This solves the issues raised above but has the dis-advantage of not matching the way structured cloning works or the "records vs. classes" semantics TC-39 seems to have settled on with the platform.

#### 4. create a new `record<T,T>` type, that covers open, half-open, and closed dictionaries and that never reaches into the prototype chain. Deprecate existing dictionaries and migrate APIs to this new type.

As dictionaries are passed by value (and not by reference), this can be explained pretty much the same way structured cloning is.

**Benefits:** consistency across the different open, half-open and closed dictionary types, structured cloning and the way TC-39 envisions records vs. classes.

**Issues:** It is possible that not reaching into the prototypes of dictionaries _at all_ would stump developers even more than the possible discrepancy between types in the current proposal. This would require migrating existing APIs currently using dictionaries to this new type. This is arguably a lot of work and might cause subtle breakage of the Web. It would also require being explicit about things we get implicitly with dictionaries such as in the `new Request(url, currentRequest)` example given above. So the [Request [[Constructor]]](https://fetch.spec.whatwg.org/#request-class) would need to be rewritten as:

```webidl
typedef (RequestInit or Request) RequestOptions;

[Constructor(RequestInfo input, optional RequestOptions init), Exposed=(Window,Worker)]
interface Request {
  // ...
};
```

### TL;DR

I'm concerned about the Web developer experience of having APIs that look the same but behave slightly differently. I also think that, as much as possible, we should avoid having different types for things that look exactly the same on the Web developer side. I'd love to have a few more eyeballs on this issue, especially folks with a Webdev background. I might also be overthinking this, and maybe @jyasskin's intuition that this is a non-issue is the right one.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/180#issuecomment-252736486