WICG / observable

Observable API proposal
https://wicg.github.io/observable/
Other
543 stars 12 forks source link

`SubscriptionObserver` members should be optional explicitly #152

Closed tetsuharuohzeki closed 1 week ago

tetsuharuohzeki commented 1 week ago

By the current spec, SubscriptionObserver's members are treated optional actually in https://wicg.github.io/observable/#ref-for-dictdef-subscriptionobserver%E2%91%A1. But WebIDL is not so. It's ambiguous. https://wicg.github.io/observable/#dictdef-subscriptionobserver

They should be optional explicitly, I think.

domfarolino commented 1 week ago

But WebIDL is not so. It's ambiguous.

I don't think so? Web IDL is very clear that all dictionary members that are not marked as required are optional. I don't think the optional keyword can be used explicitly in dictionary syntax in Web IDL anyways. At least if it can, I never really see it (example). I don't think we need to do anything here.

tetsuharuohzeki commented 1 week ago

At least if it can, I never really see it (example).

Let's see https://dom.spec.whatwg.org/#dictdef-customeventinit

domfarolino commented 1 week ago

Let's see https://dom.spec.whatwg.org/#dictdef-customeventinit

I'm not sure I understand your point. No optional keyword is expressed there. In that spec, they just define a default value. I don't think we need one here is all.

tetsuharuohzeki commented 1 week ago

Let's see https://dom.spec.whatwg.org/#dictdef-customeventinit

I'm not sure I understand your point. No optional keyword is expressed there. In that spec, they just define a default value. I don't think we need one here is all.

@domfarolino

Did you know https://webidl.spec.whatwg.org/#ref-for-dictionary-member-optional%E2%91%A0 ?


The step 3 of https://wicg.github.io/observable/#observable-subscribe-to-an-observable says "If observer’s next is not null", so I'm talking about that the following code would be more nice to express what its would be.

dictionary SubscriptionObserver {
  SubscriptionObserverCallback next = null;
  SubscriptionObserverCallback error = null;
  VoidFunction complete = null;
};

How about do you think?

domfarolino commented 1 week ago

Hmm, I'm thinking instead of null checking in the spec, and just having all of these members default to null when they are not included, we should keep the dictionary IDL as-is, and then change the null-checks (aka this here, and this here for example) to contains checks instead. So it would be if |observer|[{{SubscriptionObserver/next}}] [=map/exists=], then foo. I think that's more idiomatic and a much more common pattern, especially if we don't need the values to default to null or anything in particular.