Skip to content

Conversation

TimvdLippe
Copy link
Contributor

@TimvdLippe TimvdLippe commented Sep 13, 2025

The getter steps 1 specify to return this.signal. Its signal is nullable 2, but it is always initialized in the constructor and when cloning.


Preview | Diff

The getter steps [1] specify to return `this.signal`.
Its signal is nullable [2] and therefore the getter
should also be nullable.

[1]: https://fetch.spec.whatwg.org/#dom-request-signal
[2]: https://fetch.spec.whatwg.org/#request-signal
@TimvdLippe
Copy link
Contributor Author

This seems editorial to me, but I am actually not sure. Do let me know if this does require test coverage and all if it turns out it is normative.

@annevk
Copy link
Member

annevk commented Sep 15, 2025

I don't think this is correct. While it does start out as null, it is always initialized. By the time the getter returns it will always get an AbortSignal object.

@TimvdLippe
Copy link
Contributor Author

Hm, I am implementing it in Servo atm and it does return null sometimes. I haven't been able to figure out why though.

Should we maybe update the steps to assert it is not null and then return this.signal? That makes the invariant explicit

@annevk
Copy link
Member

annevk commented Sep 15, 2025

I think IDL is that assert. And in a future world the Web IDL specification should have such a generic assert in its getter wrapper. (That what the specification returns matches what it needs to return, or at least matches that post any Infra-to-IDL-to-JS-type conversion.)

@TimvdLippe
Copy link
Contributor Author

Will investigate why Servo is running into the assert and let you know if it is an implementation issue or not.

@TimvdLippe
Copy link
Contributor Author

Seems like I missed implementing the parts of cloning the request. Now it works correctly and we always return a non-null value. Should we add a note about this or do you think that's too much?

@annevk
Copy link
Member

annevk commented Sep 16, 2025

A note for the getter? I suppose that's reasonable.

@TimvdLippe TimvdLippe changed the title Mark AbortSignal as nullable Add note that signal is always initialized Sep 17, 2025
@TimvdLippe TimvdLippe requested a review from annevk September 18, 2025 13:54
@annevk annevk merged commit ed04423 into whatwg:main Sep 18, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants