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

service worker client definition is redundant #1046

Closed
isonmad opened this issue Jan 3, 2017 · 7 comments
Closed

service worker client definition is redundant #1046

isonmad opened this issue Jan 3, 2017 · 7 comments

Comments

@isonmad
Copy link
Contributor

isonmad commented Jan 3, 2017

8b483b0 changed the definition of a service worker client to "a type of <a>environment</a> or <a>environment settings object</a>" which is redundant, since every environment settings object is itself also an environment, so this confusingly implies they don't have an is-a relationship. Also, the 'type of' wording from #832 (comment) c11942b seems redundant and potentially confusing, isn't it more straightforward to just say "a service worker client is an environment [that...]"?

@jungkees
Copy link
Collaborator

jungkees commented Jan 3, 2017

isn't it more straightforward to just say "a service worker client is an environment [that...]"?

I'm not sure if we can assume that kind of a polymorphic behavior in the spec language.

I'm actually curious if we can make a service worker client just be a type alias to the environment (i.e the base type) and use it to dynamically reference the states depending on the actual type (either environment or environment settings object) in runtime. @annevk, @domenic, would it make sense?

@jungkees
Copy link
Collaborator

jungkees commented Jan 3, 2017

Note that we have some places in the algorithms where the logic branches depending on the types: 4ce01b5#diff-27b79860afe28f01aed4f1f6228367faR1278. (In this particular case, an environment doesn't make sense for checking of the secure context.) To deal with this sort of requirements with polymorphic approach, we'd have to define the algorithms on the base type and override them on the sub type (assuming the polymorphic behavior works for the specs in the first place.)

@isonmad
Copy link
Contributor Author

isonmad commented Apr 1, 2017

I'm not sure if we can assume that kind of a polymorphic behavior in the spec language.

Like @annevk said in #1045 (comment) that's already endemic to the specs.

To deal with this sort of requirements with polymorphic approach, we'd have to define the algorithms on the base type and override them on the sub type

Except you don't really need virtual function esque overrides to do that at all. Just do the equivalent of if (x instanceof SubType) {...} else {/* must be SuperType */...}. As long as you put the if-else-conditions in order from most-specialized to most-general types (which isn't the case right now, which is #1045), it works like you'd expect.

@jungkees
Copy link
Collaborator

jungkees commented Apr 3, 2017

Just discussed with @domenic in the f2f. We agreed what @isonmad suggested in the above comment. I'll try to update the spec by using the base type with the most-specialized-to-most-general-types language.

jungkees added a commit that referenced this issue Apr 7, 2017
This changes the definition of the service worker client from type of an
environment or an environment settings object to just an environment. As
an environment settings object *is-a* environment, this rewrites the
references to those objects without using the explicit *type of*
language. Instead, this changes to use if-else statments that check
types from most-specialized to most-general order.

Fixes #1045 #1046
@jungkees
Copy link
Collaborator

jungkees commented Apr 7, 2017

Closed by b67c445. Thanks @isonmad! I'd like to add you to acknowledgements if you're okay. Can you provide your name if so?

@jungkees jungkees closed this as completed Apr 7, 2017
@annevk
Copy link
Member

annevk commented Apr 7, 2017

@jungkees we credit isonmad as "isonmad" in various WHATWG standards. I think that's their preference.

@jungkees
Copy link
Collaborator

I added "isonmad" to the acknowledgements. Thanks for help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants