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

Spec the toArray() operator #97

Merged
merged 3 commits into from
Jan 16, 2024
Merged

Spec the toArray() operator #97

merged 3 commits into from
Jan 16, 2024

Conversation

domfarolino
Copy link
Collaborator

@domfarolino domfarolino commented Jan 5, 2024

This PR finishes spec'ing the toArray() operator, as a warm-up for the rest.


Preview | Diff

@domfarolino domfarolino requested a review from benlesh January 5, 2024 17:23
@domfarolino
Copy link
Collaborator Author

@benlesh Would you be able to take a look at this and confirm that the semantics look right to you?

@domfarolino
Copy link
Collaborator Author

I'm trying to see why PR preview isn't working on this PR. Closing and re-opening to see if that triggers it.

@domfarolino domfarolino closed this Jan 5, 2024
@domfarolino domfarolino reopened this Jan 5, 2024
spec.bs Outdated Show resolved Hide resolved
@domfarolino domfarolino merged commit cedca3d into master Jan 16, 2024
2 checks passed
@domfarolino domfarolino deleted the toArray-operator branch January 16, 2024 14:29
github-actions bot added a commit that referenced this pull request Jan 16, 2024
SHA: cedca3d
Reason: push, by domfarolino

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
aarongable pushed a commit to chromium/chromium that referenced this pull request Aug 23, 2024
After the spec discussion in
WICG/observable#97 (comment), the
remaining Promise-returning operator implementations did not include
the active document check that Observer#toArray() originally did, since
it was implemented before this discussion took place.

This CL simplifiefs `toArray()` by removing the check, since it is not
necessary. As a side effect, this will close
https://crbug.com/328281225.

R=jarhar

Bug: 40282760, 328281225
Change-Id: I0e052d36b9e6583adb6a63f47136c96b86547e17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5796962
Reviewed-by: Joey Arhar <[email protected]>
Commit-Queue: Dominic Farolino <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1346030}
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.

2 participants