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

OpenEndedDictionary clarification #164

Closed
mseddon opened this issue Nov 29, 2015 · 24 comments
Closed

OpenEndedDictionary clarification #164

mseddon opened this issue Nov 29, 2015 · 24 comments

Comments

@mseddon
Copy link

mseddon commented Nov 29, 2015

In the spec we are told:

OpenEndedDictionary is a future IDL construct. Expect it to be used as such:

var meta = { "Content-Type": "text/xml", "Breaking-Bad": "<3" }
new Headers(meta)

Sadly the currently linked IDL draft does not specify what this means. Can someone clarify how it differs from a regular dictionary?

@annevk
Copy link
Member

annevk commented Nov 30, 2015

Sorry, I was hoping we'd have this resolved by now.

@heycam, when will https://www.w3.org/Bugs/Public/show_bug.cgi?id=27008 be resolved in some way? Is @bzbarsky's solution of "optional object" plus prose really the best we can do?

@heycam
Copy link

heycam commented Nov 30, 2015

I think that's the most straightforward thing to do. I'll reply in the bug.

@bzbarsky
Copy link

@annevk My solution is the best we can do for the semantics Domenic described in that bug. It's not clear to me whether those are the semantics you actually want (and in particular he seems to be trying to avoid coercions in ways that I'm not sure the spec should actually avoid in this case).

@bzbarsky
Copy link

@annevk Oh, and the other slightly open question is what the behavior should actually be. As in, Domenic's semantics involve calling the actual "append" method (which may be overridden by the web page, subclassed, etc) whereas the current spec language is calling the internal "append" method instead. This difference is super-important: if you're going to call the internal method, then of course you want to do the type coercion before you do that, but if you're going to call this.append then of course you don't want to do the type coercion before doing so.

@annevk
Copy link
Member

annevk commented Nov 30, 2015

@domenic, what do you recommend?

@annevk
Copy link
Member

annevk commented Nov 30, 2015

(@mseddon, how would you like to be acknowledged?)

@mseddon
Copy link
Author

mseddon commented Nov 30, 2015

@annevk I am unsure what you mean by that?

@annevk
Copy link
Member

annevk commented Nov 30, 2015

I attempt to acknowledge each contribution, however minor, here: https://fetch.spec.whatwg.org/#acknowledgments.

@annevk
Copy link
Member

annevk commented Nov 30, 2015

@heycam I left a suggestion in that bug for how this could be something that IDL does for you automatically.

@mseddon
Copy link
Author

mseddon commented Nov 30, 2015

Oh, I see. Thanks :)

My name is Matt Seddon, although I would also very much like to stress that I am really reporting this issue on behalf of Henry Story, @bblfish. He was the one who actually discovered the omission, I merely verified and reported it.

@bblfish
Copy link

bblfish commented Nov 30, 2015

We're just doing a large commit of the Fetch API to scala.js, which involves trying to take into account all or as many as possible of the types that are in defined in the spec. This is how we came across this. Here is the PR https://github.com/scala-js/scala-js-dom/pull/177/files

Currently I have OpenEndedDictionary defined as

 /**
+   * see [[https://fetch.spec.whatwg.org/#headers-class ¶6.1 Headers class]] in
+   * whatwg Fetch spec.
+   * also see: [[https://github.com/whatwg/fetch/issues/164 issue 164]] in Fetch
+   * API git repo, as this is not clearly defined
+   */
+  type OpenEndedDictionary[T] = js.Dictionary[T]

@mseddon has more experience with this so he is helping review my code to help out @sjrd who wrote the compiler, but he is using most of the infrastructure and compiler put together by Martin Oderski, who also built on the work of James Gosling's java virtual machine, who built on work in OO programming, etc.. If we go back far enough we'll have a lot of people to thank :-)

@domenic
Copy link
Member

domenic commented Nov 30, 2015

@annevk Oh, and the other slightly open question is what the behavior should actually be. As in, Domenic's semantics involve calling the actual "append" method

This would be preferable, for the same reason as Map and Set call the actual append method. It makes these classes have a reasonable subclassing protocol, that allows you to override a single method (append) and have that behavior propagated to all places that append. Example:

class HeadersPrefixed extends Headers {
  append(name, value) {
    if (!someWhitelist.has(name)) {
      super.append("myapp-" + name, value);
    } else {
      super.append(name, value);
    }
  }
}

// fill a HeadersPrefixed instance, and then pass it to `new Request(...)` or something.

However, looking throug the spec I'm not sure this is sufficient to give reasonable subclassing. For example you would probably need to make .set() delegate to .delete() + .append() public APIs. And there might be other places in the spec that append to a Headers instance that fail to go through the public API. (But, maybe those only act on internal "header list"s, not on Headers instances? I can't quite tell.)

So maybe we should just give up on this principle of Headers having a reasonable subclassing protocol. Most DOM APIs don't, after all. And this might not be the right place to start taking a stand. In that case calling the internal append algorithm would be OK.

@mseddon
Copy link
Author

mseddon commented Nov 30, 2015

From reading the referenced w3c issue, it appears that OpenEndedDictionary also refers to a coercion from iterable (and possibly other?) types.

Unfortunately it seems the issue arrives to the discussion quite late- is there any other background we can refer to for this? I understand the semantics are really not yet pinned down and we will wait until everything is ratified for a 'final' implementation, but it may be less cognitive overhead our end if we can understand the root motivation for this new IDL definition.

@bzbarsky
Copy link

bzbarsky commented Dec 1, 2015

The idea of OpenEndedDictionary, in general, is to capture the idea of something like an options object (i.e. dictionary) but where the set of key names is not known ahead of time (but the keys are known to be strings, and the values are all known to be some type, possibly a union type).

There are some specs that want this sort of thing (the Headers spec, some of the animations API stuff, though the latter has some fixed set of keys and additional open-ended ones).

@annevk
Copy link
Member

annevk commented Feb 15, 2016

How likely is it that IDL will define OpenEndedDictionary? It seems Gecko implements this using something called MozMap, which I assume is similar. Does anyone know what Chrome does?

If we can just keep the complexity in IDL I'd much prefer that I think over writing more ES-prose.

@annevk
Copy link
Member

annevk commented May 4, 2016

@heycam see IDL question above about OpenEndedDictionary. Note that this also relates to an addition @domenic made to IDL that has thus far not been used (forgot the name).

@heycam
Copy link

heycam commented May 5, 2016

Are there any other specs apart from Fetch using it? I agree it would be simpler just to support this in IDL.

(We were going to use OpenEndedDictionary in Web Animations for element.animate(elt, { color: ["red", "green"], fontSize: ["10px", "20px"] }) but ended up doing it in prose. I'll see if I can make the IDL feature its behaviour, if it makes sense.)

@annevk
Copy link
Member

annevk commented May 5, 2016

I'm not sure, I guess you could grep the Gecko codebase easier than I can for MozMap.

@bzbarsky
Copy link

bzbarsky commented May 5, 2016

http://mxr.mozilla.org/mozilla-central/search?string=mozmap&find=webidl&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=mozilla-central says we use MozMap in Headers and that's it, for things that are actually standardized.

@annevk
Copy link
Member

annevk commented Aug 12, 2016

@heycam FWIW, I'd like to use something like this elsewhere too, e.g., URLSearchParams.

@annevk
Copy link
Member

annevk commented Sep 27, 2016

What really needs to be fixed here is https://www.w3.org/Bugs/Public/show_bug.cgi?id=20158 by the way. That's what we're waiting on. https://www.w3.org/Bugs/Public/show_bug.cgi?id=27008 no longer seems like the correct approach. Note that https://www.w3.org/Bugs/Public/show_bug.cgi?id=26190 also needs to be fixed for Headers to work properly.

@tobie
Copy link

tobie commented Sep 27, 2016

I'll make that top of list and start working on it by next week hopefully.

@TimothyGu
Copy link
Member

Currently the semantics of this type is a bit confused even among browser vendors. Take for example, the following code fragment:

const a = Object.create({
  three: 'three'
});
Object.defineProperties(a, {
  one: {enumerable: true, value: 'one'},
  two: {enumerable: false, value: 'two'}
});
const h = new Headers(a);
console.log(h.get('one'));
console.log(h.get('two'));
console.log(h.get('three'));

On Chrome 54.0.2840.34 (Official Build) beta (64-bit), the code results in

one
null
three

On Firefox 49.0b1:

one
null
null

Using the albeit outdated github/fetch polyfill:

one
two
null

It'd be the best if this issue could be resolved speedily. Thanks

@tobie
Copy link

tobie commented Oct 10, 2016

@TimothyGu trying to address this issue here: whatwg/webidl#180 (comment).

TimothyGu added a commit to TimothyGu/fetch-1 that referenced this issue Nov 5, 2016
annevk pushed a commit to TimothyGu/fetch-1 that referenced this issue Nov 15, 2016
annevk pushed a commit to TimothyGu/fetch-1 that referenced this issue Nov 23, 2016
annevk pushed a commit that referenced this issue Nov 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants