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

pass the mode directly as a single string option instead of 2 bools #26

Merged
merged 3 commits into from
Jun 21, 2024
+35 −71
Merged

pass the mode directly as a single string option instead of 2 bools #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
811b38a
pass the mode directly as a single string option instead of 2 bools
michaelficarra Jun 21, 2024
beb17d9
update polyfill
michaelficarra Jun 21, 2024
f14d159
replace undefined before testing for valid mode
michaelficarra Jun 21, 2024
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
65 changes: 15 additions & 50 deletions spec.emu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,13 @@ copyright: false
<emu-alg>
1. If _iterables_ is not an Object, throw a *TypeError* exception.
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _longest_ be ? GetOption(_options_, *"longest"*, ~boolean~, ~empty~, *false*).
1. Let _strict_ be ? GetOption(_options_, *"strict"*, ~boolean~, ~empty~, *false*).
1. If _longest_ is *true* and _strict_ is *true*, throw a *TypeError* exception.
1. Let _mode_ be ~shortest~.
1. Let _mode_ be ? Get(_options_, *"mode"*).
1. If _mode_ is *undefined*, set _mode_ to *"shortest"*.
1. If _mode_ is not one of *"shortest"*, *"longest"*, or *"strict"*, throw a *TypeError* exception.
1. Let _paddingOption_ be *undefined*.
1. If _longest_ is *true*, then
1. Set _mode_ to ~longest~.
1. If _mode_ is *"longest"*, then
1. Set _paddingOption_ to ? Get(_options_, *"padding"*).
1. If _paddingOption_ is not *undefined* and _paddingOption_ is not an Object, throw a *TypeError* exception.
1. If _strict_ is *true*, set _mode_ to ~strict~.
1. Let _iters_ be a new empty List.
1. Let _padding_ be a new empty List.
1. Let _inputIter_ be ? GetIterator(_iterables_, ~sync~).
Expand All @@ -37,7 +34,7 @@ copyright: false
1. IfAbruptCloseIterators(_iter_, the list-concatenation of « _inputIter_ » and _iters_).
1. Append _iter_ to _iters_.
1. Let _iterCount_ be the number of elements in _iters_.
1. If _mode_ is ~longest~, then
1. If _mode_ is *"longest"*, then
1. If _paddingOption_ is *undefined*, then
1. Perform the following steps _iterCount_ times:
1. Append *undefined* to _padding_.
Expand Down Expand Up @@ -70,16 +67,13 @@ copyright: false
<emu-alg>
1. If _iterables_ is not an Object, throw a *TypeError* exception.
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _longest_ be ? GetOption(_options_, *"longest"*, ~boolean~, ~empty~, *false*).
1. Let _strict_ be ? GetOption(_options_, *"strict"*, ~boolean~, ~empty~, *false*).
1. If _longest_ is *true* and _strict_ is *true*, throw a *TypeError* exception.
1. Let _mode_ be ~shortest~.
1. Let _mode_ be ? Get(_options_, *"mode"*).
1. If _mode_ is *undefined*, set _mode_ to *"shortest"*.
1. If _mode_ is not one of *"shortest"*, *"longest"*, or *"strict"*, throw a *TypeError* exception.
1. Let _paddingOption_ be *undefined*.
1. If _longest_ is *true*, then
1. Set _mode_ to ~longest~.
1. If _mode_ is *"longest"*, then
1. Set _paddingOption_ to ? Get(_options_, *"padding"*).
1. If _paddingOption_ is not *undefined* and _paddingOption_ is not an Object, throw a *TypeError* exception.
1. If _strict_ is *true*, set _mode_ to ~strict~.
1. Let _iters_ be a new empty List.
1. Let _padding_ be a new empty List.
1. Let _allKeys_ be ? _iterables_.[[OwnPropertyKeys]]().
Expand All @@ -103,7 +97,7 @@ copyright: false
1. IfAbruptCloseIterators(_iter_, _iters_).
1. Append _iter_ to _iters_.
1. Let _iterCount_ be the number of elements in _iters_.
1. If _mode_ is ~longest~, then
1. If _mode_ is *"longest"*, then
1. If _paddingOption_ is *undefined*, then
1. Perform the following steps _iterCount_ times:
1. Append *undefined* to _padding_.
Expand All @@ -125,7 +119,7 @@ copyright: false
<h1>
IteratorZip (
_iters_: a List of Iterator Records,
_mode_: either ~shortest~, ~longest~, or ~strict~,
_mode_: either *"shortest"*, *"longest"*, or *"strict"*,
_padding_: a List of ECMAScript language values,
_finishResults_: an Abstract Closure that takes a List of ECMAScript values and returns an ECMAScript value,
): a Generator
Expand All @@ -142,7 +136,7 @@ copyright: false
1. Assert: _openIters_ is not empty.
1. For each integer _i_ such that 0 ≤ _i_ &lt; _iterCount_, in ascending order, do
1. If _iters_[_i_] is *null*, then
1. Assert: _mode_ is ~longest~.
1. Assert: _mode_ is *"longest"*.
1. Let _result_ be _padding_[_i_].
1. Else,
1. Let _result_ be Completion(IteratorStepValue(_iters_[_i_])).
Expand All @@ -152,9 +146,9 @@ copyright: false
1. Set _result_ to ! _result_.
1. If _result_ is ~done~, then
1. Remove _iters_[_i_] from _openIters_.
1. If _mode_ is ~shortest~, then
1. If _mode_ is *"shortest"*, then
1. Return ? IteratorCloseAll(_openIters_, NormalCompletion(*undefined*)).
1. Else if _mode_ is ~strict~, then
1. Else if _mode_ is *"strict"*, then
1. If _i_ ≠ 0, then
1. Return ? IteratorCloseAll(_openIters_, ThrowCompletion(a newly created *TypeError* object)).
1. For each integer _k_ such that 1 ≤ _k_ &lt; _iterCount_, in ascending order, do
Expand All @@ -170,7 +164,7 @@ copyright: false
1. Return ? IteratorCloseAll(_openIters_, ThrowCompletion(a newly created *TypeError* object)).
1. Return *undefined*.
1. Else,
1. Assert: _mode_ is ~longest~.
1. Assert: _mode_ is *"longest"*.
1. If _openIters_ is empty, return *undefined*.
1. Set _iters_[_i_] to *null*.
1. Set _result_ to _padding_[_i_].
Expand Down Expand Up @@ -263,34 +257,5 @@ copyright: false
1. Throw a *TypeError* exception.
</emu-alg>
</emu-clause>

<emu-clause id="sec-getoption" type="abstract operation">
<h1>
GetOption (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use GetOption for the mode option? it would handle the default in a single step.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetOption (taken from 402) coerces the value based on the third parameter. We have a normative convention to no longer do that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but that suggests we should enhance GetOption to skip coercing, since we'll be using GetOption in Temporal anyways.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure, but again, that's in 402, so not really this proposal's problem. We'll have to figure out how to handle that editorially when merging temporal, since its coercion behaviour is presumably not changing to match our new conventions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, but its coercion behavior should definitely be changing to match our new conventions - it hasn't shipped on the web yet so there's no reason not to do so.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're going to have to bring that up with the temporal people, not me. Also, they're not the only users. GetOption, with its current coercing behaviour, is used pretty extensively throughout 402 today: https://tc39.es/ecma402/#sec-getoption

_options_: an Object,
_property_: a property key,
_type_: ~boolean~ or ~string~,
_values_: ~empty~ or a List of ECMAScript language values,
_default_: ~required~ or an ECMAScript language value,
): either a normal completion containing an ECMAScript language value or a throw completion
</h1>
<dl class="header">
<dt>description</dt>
<dd>It extracts the value of the specified property of _options_, converts it to the required _type_, checks whether it is allowed by _values_ if _values_ is not ~empty~, and substitutes _default_ if the value is *undefined*.</dd>
</dl>
<emu-alg>
1. Let _value_ be ? Get(_options_, _property_).
1. If _value_ is *undefined*, then
1. If _default_ is ~required~, throw a *RangeError* exception.
1. Return _default_.
1. If _type_ is ~boolean~, then
1. Set _value_ to ToBoolean(_value_).
1. Else,
1. Assert: _type_ is ~string~.
1. Set _value_ to ? ToString(_value_).
1. If _values_ is not ~empty~ and _values_ does not contain _value_, throw a *RangeError* exception.
1. Return _value_.
</emu-alg>
</emu-clause>
</emu-clause>

23 changes: 11 additions & 12 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ if (typeof Iterator === 'undefined' || Iterator == null) {
globalThis.Iterator = {};
}

const DEFAULT_FILLER = void 0;
const DEFAULT_FILLER = undefined;

function getIteratorFlattenable(obj: any, stringHandling: 'iterate-strings' | 'reject-strings'): Iterator<unknown> {
if (Object(obj) !== obj) {
Expand All @@ -30,17 +30,14 @@ function getOwnEnumerablePropertyKeys<O extends Object>(obj: O): Array<keyof O>
}

interface ZipShortestOptions {
longest?: false,
strict?: false,
mode?: 'shortest',
}
interface ZipLongestOptions<F> {
longest: true,
strict?: false,
mode: 'longest',
padding?: F,
}
interface ZipStrictOptions {
longest?: false,
strict: true,
mode: 'strict',
};
type ZipOptions<F> = ZipShortestOptions | ZipLongestOptions<F> | ZipStrictOptions;

Expand All @@ -53,12 +50,14 @@ type IterateesOfTupleOfIterables<T extends readonly IteratorOrIterable<unknown>[
type Mode = 'shortest' | 'longest' | 'strict';

function getMode(options: ZipOptions<any>): Mode {
let longest = (options as ZipOptions<never>).longest;
let strict = (options as ZipOptions<never>).strict;
if (longest && strict) {
let mode = (options as ZipOptions<never>).mode;
if (mode === undefined) {
mode = 'shortest';
}
if (mode !== 'shortest' && mode !== 'longest' && mode !== 'strict') {
throw new TypeError;
}
return longest ? 'longest' : (strict ? 'strict' : 'shortest');
return mode as Mode;
}

function zipToArrays(p: readonly [], o?: ZipOptions<Iterable<unknown>>): IterableIterator<never>
Expand Down Expand Up @@ -213,4 +212,4 @@ Object.defineProperty(Iterator, 'zipToObjects', {
writable: true,
enumerable: false,
value: zipToObjects,
});
});
18 changes: 9 additions & 9 deletions test/index.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ test('basic positional', async t => {
assert.deepEqual(Array.from(Iterator.zipToArrays([
[0],
[1, 2],
], { longest: true })), [
], { mode: 'longest' })), [
[0, 1],
[undefined, 2],
]);
Expand All @@ -43,7 +43,7 @@ test('basic positional', async t => {
Iterator.zipToArrays([
[0],
[1, 2],
], { strict: true });
], { mode: 'strict' });
assert.throws(() => {
Array.from(result);
}, RangeError);
Expand Down Expand Up @@ -80,7 +80,7 @@ test('basic named', async t => {
assert.deepEqual(Array.from(Iterator.zipToObjects({
a: [0],
b: [1, 2],
}, { longest: true })), [
}, { mode: 'longest' })), [
{ a: 0, b: 1 },
{ a: undefined, b: 2 },
]);
Expand All @@ -91,7 +91,7 @@ test('basic named', async t => {
Iterator.zipToObjects({
a: [0],
b: [1, 2],
}, { strict: true });
}, { mode: 'strict' });
assert.throws(() => {
Array.from(result);
}, RangeError);
Expand All @@ -105,7 +105,7 @@ test('padding', async t => {
assert.deepEqual(Array.from(Iterator.zipToArrays([
[0],
[1, 2, 3],
], { longest: true, padding })), [
], { mode: 'longest', padding })), [
[0, 1],
[padding[0], 2],
[padding[0], 3],
Expand All @@ -116,7 +116,7 @@ test('padding', async t => {
[1, 2, 3],
[4, 5],
[],
], { longest: true, padding })), [
], { mode: 'longest', padding })), [
[0, 1, 4, padding[3]],
[padding[0], 2, 5, padding[3]],
[padding[0], 3, padding[2], padding[3]],
Expand All @@ -139,7 +139,7 @@ test('padding', async t => {
assert.deepEqual(Array.from(Iterator.zipToObjects({
a: [0],
b: [1, 2, 3],
}, { longest: true, padding })), [
}, { mode: 'longest', padding })), [
{ a: 0, b: 1 },
{ a: A_PADDING, b: 2 },
{ a: A_PADDING, b: 3 },
Expand All @@ -150,10 +150,10 @@ test('padding', async t => {
b: [1, 2, 3],
c: [4, 5],
d: [],
}, { longest: true, padding })), [
}, { mode: 'longest', padding })), [
{ a: 0, b: 1, c: 4, d: D_PADDING },
{ a: A_PADDING, b: 2, c: 5, d: D_PADDING },
{ a: A_PADDING, b: 3, c: C_PADDING, d: D_PADDING },
]);
});
});
});