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.

Sign up for GitHub

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

Jump to bottom

DOM: Fix Observable#from() [Symbol.iterator] semantics (1/2) #47881

Merged
merged 1 commit into from
Aug 30, 2024
Merged

DOM: Fix Observable#from() [Symbol.iterator] semantics (1/2) #47881

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
277 changes: 233 additions & 44 deletions dom/observable/tentative/observable-from.any.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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(dom@chromium.org): Add another test like the above, but for
// `[Symbol.asyncIterator] = null` falling back to `[Symbol.iterator]`
// conversion.