Skip to content

Commit

Permalink
LibJS: Apply normative change to Array.fromAsync to avoid double cons…
Browse files Browse the repository at this point in the history
…truct

Apply a normative fix to Array.fromAsync fixed in:

tc39/proposal-array-from-async#41

This avoids a double construction when Array.fromAsync is given an array
like object.
  • Loading branch information
shannonbooth committed Aug 28, 2023
1 parent b0eea51 commit 630f046
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 21 deletions.
40 changes: 21 additions & 19 deletions Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,39 +335,39 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from_async)
using_sync_iterator = TRY(async_items.get_method(vm, vm.well_known_symbol_iterator()));
}

GCPtr<Object> array;

// e. If IsConstructor(C) is true, then
if (constructor.is_constructor()) {
// i. Let A be ? Construct(C).
array = TRY(JS::construct(vm, constructor.as_function()));
}
// f. Else,
else {
// i. Let A be ! ArrayCreate(0).
array = MUST(Array::create(realm, 0));
}

// g. Let iteratorRecord be undefined.
// e. Let iteratorRecord be undefined.
Optional<IteratorRecord> iterator_record;

// h. If usingAsyncIterator is not undefined, then
// f. If usingAsyncIterator is not undefined, then
if (using_async_iterator) {
// i. Set iteratorRecord to ? GetIterator(asyncItems, async, usingAsyncIterator).
// FIXME: The Array.from proposal is out of date - it should be using GetIteratorFromMethod.
iterator_record = TRY(get_iterator_from_method(vm, async_items, *using_async_iterator));
}
// i. Else if usingSyncIterator is not undefined, then
// g. Else if usingSyncIterator is not undefined, then
else if (using_sync_iterator) {
// i. Set iteratorRecord to ? CreateAsyncFromSyncIterator(GetIterator(asyncItems, sync, usingSyncIterator)).
// FIXME: The Array.from proposal is out of date - it should be using GetIteratorFromMethod.
iterator_record = create_async_from_sync_iterator(vm, TRY(get_iterator_from_method(vm, async_items, *using_sync_iterator)));
}

// j. If iteratorRecord is not undefined, then
// h. If iteratorRecord is not undefined, then
if (iterator_record.has_value()) {
// i. Let k be 0.
// ii. Repeat,
GCPtr<Object> array;

// i. If IsConstructor(C) is true, then
if (constructor.is_constructor()) {
// 1. Let A be ? Construct(C).
array = TRY(JS::construct(vm, constructor.as_function()));
}
// ii. Else,
else {
// i. Let A be ! ArrayCreate(0).
array = MUST(Array::create(realm, 0));
}

// iii. Let k be 0.
// iv. Repeat,
for (size_t k = 0;; ++k) {
// 1. If k ≥ 2^53 - 1, then
if (k >= MAX_ARRAY_LIKE_INDEX) {
Expand Down Expand Up @@ -471,6 +471,8 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from_async)
// iii. Let len be ? LengthOfArrayLike(arrayLike).
auto length = TRY(length_of_array_like(vm, array_like));

GCPtr<Object> array;

// iv. If IsConstructor(C) is true, then
if (constructor.is_constructor()) {
// 1. Let A be ? Construct(C, « 𝔽(len) »).
Expand Down
24 changes: 22 additions & 2 deletions Userland/Libraries/LibJS/Tests/builtins/Array/Array.fromAsync.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ test("length is 1", () => {
});

describe("normal behavior", () => {
function checkResult(promise) {
function checkResult(promise, type = Array) {
expect(promise).toBeInstanceOf(Promise);
let error = null;
let passed = false;
promise
.then(value => {
expect(value instanceof Array).toBeTrue();
expect(value instanceof type).toBeTrue();
expect(value[0]).toBe(0);
expect(value[1]).toBe(2);
expect(value[2]).toBe(4);
Expand Down Expand Up @@ -57,4 +57,24 @@ describe("normal behavior", () => {
);
checkResult(promise);
});

test("does not double construct from array like object", () => {
let callCount = 0;

class TestArray {
constructor() {
callCount += 1;
}
}

let promise = Array.fromAsync.call(TestArray, {
length: 3,
0: Promise.resolve(0),
1: Promise.resolve(2),
2: Promise.resolve(4),
});

checkResult(promise, TestArray);
expect(callCount).toBe(1);
});
});

0 comments on commit 630f046

Please sign in to comment.