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

Do not assign a default value to WakeLockRequestOptions.signal #203

Closed
wants to merge 1 commit into from

Conversation

rakuco
Copy link
Member

@rakuco rakuco commented May 3, 2019

In Web IDL, the following WakeLockRequestOptions instances in JavaScript
should have the same effect (a dictionary that does not have the signal
member present):

{}
{signal: undefined}

which are different from

{signal: null}

which means a WakeLockRequestOptions dictionary with one member, signal,
whose value is null.

Setting a default value to the nullable member signal means we'd always
have a signal member in the dictionary even when we do not set it at
all (first 2 cases), which is not needed and does not make sense.


Preview | Diff

In Web IDL, the following `WakeLockRequestOptions` instances in JavaScript
should have the same effect (a dictionary that does _not_ have the `signal`
member present):

    {}
    {signal: undefined}

which are different from

    {signal: null}

which means a `WakeLockRequestOptions` dictionary with one member, `signal`,
whose value is `null`.

Setting a default value to the nullable member `signal` means we'd always
have a `signal` member in the dictionary even when we do not set it at
all (first 2 cases), which is not needed and does not make sense.
@kenchris
Copy link
Contributor

kenchris commented May 4, 2019

This original change was requested by @marcoscaceres so he probably has comments

@marcoscaceres
Copy link
Member

marcoscaceres commented May 6, 2019

Setting a default value to the nullable member signal means we'd always
have a signal member in the dictionary even when we do not set it at
all (first 2 cases), which is not needed and does not make sense.

Agree. The problem was (is?) that the algorithm prose was defaulting it to null. If the prose doesn't do that anymore, then I'm ok with this.

However, I'd still like clarity on what happens when null is set. Why have nullable at all?

@rakuco
Copy link
Member Author

rakuco commented May 6, 2019

However, I'd still like clarity on what happens when null is set. Why have nullable at all?

You have a point, I don't think this needs to be nullable at all. Most other occurrences of AbortSignal in a dictionary aren't nullable either, module Fetch's RequestInit.signal, and only because one can pass a Request to another Request's constructor.

I'll send a new PR making signal a regular member.

@rakuco rakuco closed this May 6, 2019
@rakuco rakuco deleted the abortsignal-default-value branch May 6, 2019 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants