-
Notifications
You must be signed in to change notification settings - Fork 2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Bug 1915764 [wpt PR 47881] - DOM: Fix
Observable#from()
[Symbol.ite…
…rator] semantics (1/2), a=testonly Automatic update from web-platform-tests DOM: Fix `Observable#from()` [Symbol.iterator] semantics (1/2) See WICG/observable#160, which specs the `Observable#from()` semantics, matching these tests. See also https://crbug.com/363015168 which describes the ways in which our current implementation of `Observable#from()`'s detection semantics are overbroad. This CL makes the implementation of the "detection semantics" match the desired behavior outlined in that issue, and adds a bunch of tests. [email protected] Bug: 363015168, 40282760 Change-Id: Id6cfdd45c44286b298e107635e4283b018f50aaf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5824955 Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Mason Freed <[email protected]> Cr-Commit-Position: refs/heads/main@{#1349019} -- wpt-commits: e74098ed10aa0a4fc6ad6b8d6e354895addd00d1 wpt-pr: 47881
- Loading branch information
1 parent
45efcad
commit f098e33
Showing
1 changed file
with
233 additions
and
44 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,43 +85,124 @@ test(() => { | |
"Subscribing again causes another fresh iteration on an un-exhausted iterable"); | ||
}, "from(): Iterable converts to Observable"); | ||
|
||
// The result of the @@iterator method of the converted object is called: | ||
// 1. Once on conversion (to test that the value is an iterable). | ||
// 2. Once on subscription, to re-pull the iterator implementation from the | ||
// raw JS object that the Observable owns once synchronous iteration is | ||
// about to begin. | ||
// This test, and the variants below it, test the web-observable side-effects of | ||
// converting an iterable object to an Observable. Specifically, it tracks | ||
// exactly when the %Symbol.iterator% method is *retrieved* from the object, | ||
// invoked, and what its error-throwing side-effects are. | ||
// | ||
// Even more specifically, we assert that the %Symbol.iterator% method is | ||
// retrieved a single time when converting to an Observable, and then again when | ||
// subscribing to the converted Observable. This makes it possible for the | ||
// %Symbol.iterator% method getter to change return values in between conversion | ||
// and subscription. See https://github.com/WICG/observable/issues/127 for | ||
// related discussion. | ||
test(() => { | ||
let numTimesSymbolIteratorCalled = 0; | ||
let numTimesNextCalled = 0; | ||
const results = []; | ||
|
||
const iterable = { | ||
[Symbol.iterator]() { | ||
numTimesSymbolIteratorCalled++; | ||
get [Symbol.iterator]() { | ||
results.push("[Symbol.iterator] method GETTER"); | ||
return function() { | ||
results.push("[Symbol.iterator implementation]"); | ||
return { | ||
get next() { | ||
results.push("next() method GETTER"); | ||
return function() { | ||
results.push("next() implementation"); | ||
return {value: undefined, done: true}; | ||
}; | ||
}, | ||
}; | ||
}; | ||
}, | ||
}; | ||
|
||
const observable = Observable.from(iterable); | ||
assert_array_equals(results, ["[Symbol.iterator] method GETTER"]); | ||
|
||
let thrownError = null; | ||
observable.subscribe(); | ||
assert_array_equals(results, [ | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator implementation]", | ||
"next() method GETTER", | ||
"next() implementation" | ||
]); | ||
}, "from(): [Symbol.iterator] side-effects (one observable)"); | ||
|
||
// This tests that once `Observable.from()` detects a non-null and non-undefined | ||
// `[Symbol.iterator]` property, we've committed to converting as an iterable. | ||
// If the value of that property is not callable, we don't silently move on to | ||
// the next conversion type — we throw a TypeError; | ||
test(() => { | ||
let results = []; | ||
const iterable = { | ||
[Symbol.iterator]: 10, | ||
}; | ||
|
||
let errorThrown = null; | ||
try { | ||
Observable.from(iterable); | ||
} catch(e) { | ||
errorThrown = e; | ||
} | ||
|
||
assert_true(errorThrown instanceof TypeError); | ||
assert_equals(errorThrown.message, | ||
"Failed to execute 'from' on 'Observable': @@iterator must be a " + | ||
"callable."); | ||
}, "from(): [Symbol.iterator] not callable"); | ||
|
||
test(() => { | ||
let results = []; | ||
const customError = new Error("@@iterator override error"); | ||
|
||
const iterable = { | ||
numTimesCalled: 0, | ||
|
||
// The first time this getter is called, it returns a legitimate function | ||
// that, when called, returns an iterator. Every other time it returns an | ||
// error-throwing function that does not return an iterator. | ||
get [Symbol.iterator]() { | ||
this.numTimesCalled++; | ||
results.push("[Symbol.iterator] method GETTER"); | ||
|
||
if (this.numTimesCalled === 1) { | ||
return this.validIteratorImplementation; | ||
} else { | ||
return this.errorThrowingIteratorImplementation; | ||
} | ||
}, | ||
|
||
validIteratorImplementation: function() { | ||
results.push("[Symbol.iterator implementation]"); | ||
return { | ||
next() { | ||
numTimesNextCalled++; | ||
return {value: undefined, done: true}; | ||
get next() { | ||
results.push("next() method GETTER"); | ||
return function() { | ||
results.push("next() implementation"); | ||
return {value: undefined, done: true}; | ||
} | ||
} | ||
}; | ||
} | ||
}, | ||
errorThrowingIteratorImplementation: function() { | ||
results.push("Error-throwing [Symbol.iterator] implementation"); | ||
throw customError; | ||
}, | ||
}; | ||
|
||
const observable = Observable.from(iterable); | ||
|
||
assert_equals(numTimesSymbolIteratorCalled, 1, | ||
"Observable.from(iterable) invokes the @@iterator method getter once"); | ||
assert_equals(numTimesNextCalled, 0, | ||
"Iterator next() is not called until subscription"); | ||
assert_array_equals(results, [ | ||
"[Symbol.iterator] method GETTER", | ||
]); | ||
|
||
// Override iterable's `[Symbol.iterator]` protocol with an error-throwing | ||
// function. We assert that on subscription, this method (the new `@@iterator` | ||
// implementation), is called because only the raw JS object gets stored in | ||
// the Observable that results in conversion. This raw value must get | ||
// re-converted to an iterable once iteration is about to start. | ||
const customError = new Error('@@iterator override error'); | ||
iterable[Symbol.iterator] = () => { | ||
throw customError; | ||
}; | ||
|
||
let thrownError = null; | ||
observable.subscribe({ | ||
|
@@ -130,56 +211,101 @@ test(() => { | |
|
||
assert_equals(thrownError, customError, | ||
"Error thrown from next() is passed to the error() handler"); | ||
|
||
assert_equals(numTimesSymbolIteratorCalled, 1, | ||
"Subscription re-invokes @@iterator method, which now is a different " + | ||
"method that does *not* increment our assertion value"); | ||
assert_equals(numTimesNextCalled, 0, "Iterator next() is never called"); | ||
}, "from(): [Symbol.iterator] side-effects (one observable)"); | ||
assert_array_equals(results, [ | ||
// Old: | ||
"[Symbol.iterator] method GETTER", | ||
// New: | ||
"[Symbol.iterator] method GETTER", | ||
"Error-throwing [Symbol.iterator] implementation" | ||
]); | ||
}, "from(): [Symbol.iterator] is not cached"); | ||
|
||
// Similar to the above test, but with more Observables! | ||
test(() => { | ||
const results = []; | ||
let numTimesSymbolIteratorCalled = 0; | ||
let numTimesNextCalled = 0; | ||
|
||
const iterable = { | ||
[Symbol.iterator]() { | ||
numTimesSymbolIteratorCalled++; | ||
get [Symbol.iterator]() { | ||
results.push("[Symbol.iterator] method GETTER"); | ||
return this.internalIteratorImplementation; | ||
}, | ||
set [Symbol.iterator](func) { | ||
this.internalIteratorImplementation = func; | ||
}, | ||
|
||
internalIteratorImplementation: function() { | ||
results.push("[Symbol.iterator] implementation"); | ||
return { | ||
next() { | ||
numTimesNextCalled++; | ||
return {value: undefined, done: true}; | ||
} | ||
get next() { | ||
results.push("next() method GETTER"); | ||
return function() { | ||
results.push("next() implementation"); | ||
return {value: undefined, done: true}; | ||
}; | ||
}, | ||
}; | ||
} | ||
}, | ||
}; | ||
|
||
const obs1 = Observable.from(iterable); | ||
const obs2 = Observable.from(iterable); | ||
const obs3 = Observable.from(iterable); | ||
const obs4 = Observable.from(obs3); | ||
|
||
assert_equals(numTimesSymbolIteratorCalled, 3, "Observable.from(iterable) invokes the iterator method getter once"); | ||
assert_equals(numTimesNextCalled, 0, "Iterator next() is not called until subscription"); | ||
assert_equals(obs3, obs4); | ||
|
||
assert_array_equals(results, [ | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] method GETTER", | ||
]); | ||
|
||
obs1.subscribe(); | ||
assert_array_equals(results, [ | ||
// Old: | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] method GETTER", | ||
// New: | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] implementation", | ||
"next() method GETTER", | ||
"next() implementation", | ||
]); | ||
|
||
iterable[Symbol.iterator] = () => { | ||
results.push("Error-throwing [Symbol.iterator] implementation"); | ||
throw new Error('Symbol.iterator override error'); | ||
}; | ||
|
||
let errorCount = 0; | ||
|
||
const observer = {error: e => errorCount++}; | ||
obs1.subscribe(observer); | ||
obs2.subscribe(observer); | ||
obs3.subscribe(observer); | ||
obs4.subscribe(observer); | ||
assert_equals(errorCount, 4, | ||
assert_equals(errorCount, 3, | ||
"Error-throwing `@@iterator` implementation is called once per " + | ||
"subscription"); | ||
|
||
assert_equals(numTimesSymbolIteratorCalled, 3, | ||
"Subscription re-invokes the iterator method getter once"); | ||
assert_equals(numTimesNextCalled, 0, "Iterator next() is never called"); | ||
assert_array_equals(results, [ | ||
// Old: | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] method GETTER", | ||
"[Symbol.iterator] implementation", | ||
"next() method GETTER", | ||
"next() implementation", | ||
// New: | ||
"[Symbol.iterator] method GETTER", | ||
"Error-throwing [Symbol.iterator] implementation", | ||
"[Symbol.iterator] method GETTER", | ||
"Error-throwing [Symbol.iterator] implementation", | ||
"[Symbol.iterator] method GETTER", | ||
"Error-throwing [Symbol.iterator] implementation", | ||
]); | ||
}, "from(): [Symbol.iterator] side-effects (many observables)"); | ||
|
||
test(() => { | ||
|
@@ -221,7 +347,8 @@ promise_test(async () => { | |
|
||
await promise; | ||
|
||
assert_array_equals(results, ["value", "complete()"], "Observable emits and completes after Promise resolves"); | ||
assert_array_equals(results, ["value", "complete()"], | ||
"Observable emits and completes after Promise resolves"); | ||
}, "from(): Converts Promise to Observable"); | ||
|
||
promise_test(async t => { | ||
|
@@ -352,3 +479,65 @@ test(() => { | |
assert_array_equals(results, ["from @@iterator", "complete"]); | ||
}, "from(): Promise that implements @@iterator protocol gets converted as " + | ||
"an iterable, not Promise"); | ||
|
||
// When the [Symbol.iterator] method on a given object is undefined, we don't | ||
// try to convert the object to an Observable via the iterable protocol. The | ||
// Observable specification *also* does the same thing if the [Symbol.iterator] | ||
// method is *null*. That is, in that case we also skip the conversion via | ||
// iterable protocol, and continue to try and convert the object as another type | ||
// (in this case, a Promise). | ||
promise_test(async () => { | ||
const promise = new Promise(resolve => resolve('from Promise')); | ||
assert_equals(promise[Symbol.iterator], undefined); | ||
promise[Symbol.iterator] = null; | ||
assert_equals(promise[Symbol.iterator], null); | ||
|
||
const value = await new Promise(resolve => { | ||
Observable.from(promise).subscribe(value => resolve(value)); | ||
}); | ||
|
||
assert_equals(value, 'from Promise'); | ||
}, "from(): Promise whose [Symbol.iterator] returns null converts as Promise"); | ||
|
||
// This is a more sensitive test, which asserts that even just trying to reach | ||
// for the [Symbol.iterator] method on an object whose *getter* for the | ||
// [Symbol.iterator] method throws an error, results in `Observable#from()` | ||
// rethrowing that error. | ||
test(() => { | ||
const error = new Error('thrown from @@iterator getter'); | ||
const obj = { | ||
get [Symbol.iterator]() { | ||
throw error; | ||
} | ||
} | ||
|
||
try { | ||
Observable.from(obj); | ||
assert_unreached("from() conversion throws"); | ||
} catch(e) { | ||
assert_equals(e, error); | ||
} | ||
}, "from(): Rethrows the error when Converting an object whose @@iterator " + | ||
"method *getter* throws an error"); | ||
|
||
test(() => { | ||
const obj = {}; | ||
// Non-undefined & non-null values of the `@@iterator` property are not | ||
// allowed. Specifically they fail the the `IsCallable()` test, which fails | ||
// Observable conversion. | ||
obj[Symbol.iterator] = 10; | ||
|
||
try { | ||
Observable.from(obj); | ||
assert_unreached("from() conversion throws"); | ||
} catch(e) { | ||
assert_true(e instanceof TypeError); | ||
assert_equals(e.message, | ||
"Failed to execute 'from' on 'Observable': @@iterator must be a callable."); | ||
} | ||
}, "from(): Throws 'callable' error when @@iterator property is a " + | ||
"non-callable primitive"); | ||
|
||
// TODO([email protected]): Add another test like the above, but for | ||
// `[Symbol.asyncIterator] = null` falling back to `[Symbol.iterator]` | ||
// conversion. |