[New] ES2018+: Add CreateAsyncFromSyncIterator
#105
Conversation
2018/CreateAsyncFromSyncIterator.js
Outdated
|
|
||
| // eslint-disable-next-line no-shadow | ||
| var AsyncFromSyncIteratorPrototype = OrdinaryObjectCreate(AsyncIteratorPrototype); | ||
| CreateMethodProperty(AsyncFromSyncIteratorPrototype, 'next', function next(value) { |
There was a problem hiding this comment.
Arguably, these function bodies should be wrapped in the callback to new $Promise.
|
|
||
| // https://www.ecma-international.org/ecma-262/10.0/#sec-asyncfromsynciteratorcontinuation | ||
|
|
||
| module.exports = function AsyncFromSyncIteratorContinuation(result, promiseCapability) { |
There was a problem hiding this comment.
the spec doesn't have the proper assertions here (see tc39/ecma262#2050) but i think we should write a helper and the runtime check here.
ExE-Boss
force-pushed
the
feat/es2018/create-async-from-sync-iterator
branch
from
a48d96e
to
a0eedfa
Compare
Codecov Report
@@ Coverage Diff @@
## master #105 +/- ##
==========================================
- Coverage 90.89% 90.30% -0.59%
==========================================
Files 670 673 +3
Lines 9444 9574 +130
Branches 2195 2199 +4
==========================================
+ Hits 8584 8646 +62
- Misses 860 928 +68
Continue to review full report at Codecov.
|
| throw new SyntaxError('This environment does not support Promises.'); | ||
| } | ||
|
|
||
| var asyncIterator = new AsyncFromSyncIterator(syncIteratorRecord); |
There was a problem hiding this comment.
| var asyncIterator = new AsyncFromSyncIterator(syncIteratorRecord); | |
| var asyncIterator = AsyncFromSyncIterator(syncIteratorRecord); |
i'm very confused why we'd want a constructor, or to encourage use of new.
|
|
||
| // TODO: Use %AsyncIterator.from% once it's in ECMA-262 | ||
|
|
||
| /** @type {(o: object, p: string | symbol, Desc: import('es-abstract').PropertyDescriptor) => boolean} */ |
There was a problem hiding this comment.
| /** @type {(o: object, p: string | symbol, Desc: import('es-abstract').PropertyDescriptor) => boolean} */ |
i'm not interested in jsdoc comments at this time
|
|
||
| /** @type {(o: object, p: string | symbol, Desc: import('es-abstract').PropertyDescriptor) => boolean} */ | ||
| var DefineOwnProperty = callBind( | ||
| require('./DefineOwnProperty'), |
There was a problem hiding this comment.
this should be required at module level and used here
| function IsDataDescriptor(Desc) { | ||
| return !('[[Get]]' in Desc) && !('[[Set]]' in Desc); | ||
| }, |
There was a problem hiding this comment.
this should use require('./IsDataDescriptor').
There was a problem hiding this comment.
You do realise that this is now in the helpers directory, right?
As such, there is no IsDataDescriptor file that can be imported.
There was a problem hiding this comment.
In that case, it should take IsDataDescriptor as a function argument.
| GetIntrinsic('%Object.is%', true) || function SameValue(x, y) { | ||
| if (x === y) { | ||
| // 0 === -0, but they are not identical. | ||
| if (x === 0) { | ||
| return 1 / x === 1 / y; | ||
| } | ||
| return true; | ||
| } | ||
| return $isNaN(x) && $isNaN(y); | ||
| }, |
There was a problem hiding this comment.
this should just use require('./SameValue') directly (required at module level)
There was a problem hiding this comment.
This should also be passed as a function argument.
| function FromPropertyDescriptor(Desc) { | ||
| var obj = {}; | ||
| if ('[[Value]]' in Desc) { | ||
| obj.value = Desc['[[Value]]']; | ||
| } | ||
| if ('[[Writable]]' in Desc) { | ||
| obj.writable = Desc['[[Writable]]']; | ||
| } | ||
| if ('[[Get]]' in Desc) { | ||
| obj.get = Desc['[[Get]]']; | ||
| } | ||
| if ('[[Set]]' in Desc) { | ||
| obj.set = Desc['[[Set]]']; | ||
| } | ||
| if ('[[Enumerable]]' in Desc) { | ||
| obj.enumerable = Desc['[[Enumerable]]']; | ||
| } | ||
| if ('[[Configurable]]' in Desc) { | ||
| obj.configurable = Desc['[[Configurable]]']; | ||
| } | ||
| return obj; | ||
| } |
There was a problem hiding this comment.
| function FromPropertyDescriptor(Desc) { | |
| var obj = {}; | |
| if ('[[Value]]' in Desc) { | |
| obj.value = Desc['[[Value]]']; | |
| } | |
| if ('[[Writable]]' in Desc) { | |
| obj.writable = Desc['[[Writable]]']; | |
| } | |
| if ('[[Get]]' in Desc) { | |
| obj.get = Desc['[[Get]]']; | |
| } | |
| if ('[[Set]]' in Desc) { | |
| obj.set = Desc['[[Set]]']; | |
| } | |
| if ('[[Enumerable]]' in Desc) { | |
| obj.enumerable = Desc['[[Enumerable]]']; | |
| } | |
| if ('[[Configurable]]' in Desc) { | |
| obj.configurable = Desc['[[Configurable]]']; | |
| } | |
| return obj; | |
| } | |
| FromPropertyDescriptor, |
where FromPropertyDescriptor is require('./FromPropertyDescriptor') at module level
| var CreateMethodProperty = function CreateMethodProperty(O, P, V) { | ||
| return DefineOwnProperty(O, P, { | ||
| '[[Configurable]]': true, | ||
| '[[Enumerable]]': false, | ||
| '[[Writable]]': true, | ||
| '[[Value]]': V | ||
| }); | ||
| }; |
There was a problem hiding this comment.
this should also be require('./CreateMethodProperty')
| var isObject = function (target) { | ||
| return target !== null && (typeof target === 'object' || typeof target === 'function'); | ||
| }; |
There was a problem hiding this comment.
please use the inverse of helpers/isPrimitive for this.
| || (function () { | ||
| var result = {}; | ||
| if ($toStringTag) { | ||
| DefineOwnProperty(result, $toStringTag, { | ||
| '[[Writable]]': false, | ||
| '[[Enumerable]]': false, | ||
| '[[Configurable]]': true, | ||
| '[[Value]]': 'AsyncIterator' | ||
| }); | ||
| } | ||
| if ($asyncIterator) { | ||
| CreateMethodProperty( | ||
| result, | ||
| $asyncIterator, | ||
| { | ||
| '[Symbol.asyncIterator]': function () { | ||
| return this; | ||
| } | ||
| }['[Symbol.asyncIterator]'] | ||
| ); | ||
| } | ||
| return result; | ||
| }()); |
There was a problem hiding this comment.
this should be moved to a separate helpers file, that's tested independently.
| undefined, | ||
| [{ value: unwrappedValue, done: done }] | ||
| ); | ||
| }, | ||
| promiseCapability['[[Reject]]'] | ||
| ); | ||
| }; | ||
|
|
||
| var T = function T() {}; | ||
| T.prototype = AsyncIteratorPrototype; | ||
| // eslint-disable-next-line no-shadow | ||
| var AsyncFromSyncIteratorPrototype = new T(); | ||
|
|
||
| CreateMethodProperty(AsyncFromSyncIteratorPrototype, 'next', function next(value) { | ||
| // eslint-disable-next-line no-invalid-this | ||
| var O = this; | ||
| return new Promise(function (resolve, reject) { | ||
| internalSlot.assert(O, '[[SyncIteratorRecord]]'); | ||
|
|
||
| var syncIteratorRecord = internalSlot.get(O, '[[SyncIteratorRecord]]'); | ||
| var result = $apply( | ||
| syncIteratorRecord['[[NextMethod]]'], | ||
| syncIteratorRecord['[[Iterator]]'], | ||
| [value] | ||
| ); | ||
|
|
||
| if (!isObject(result)) { | ||
| throw new $TypeError('iterator next must return an object'); | ||
| } | ||
|
|
||
| return AsyncFromSyncIteratorContinuation(result, { | ||
| '[[Resolve]]': resolve, | ||
| '[[Reject]]': reject | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| CreateMethodProperty(AsyncFromSyncIteratorPrototype, 'return', { | ||
| 'return': function (value) { | ||
| var O = this; | ||
| return new Promise(function (resolve, reject) { | ||
| internalSlot.assert(O, '[[SyncIteratorRecord]]'); | ||
|
|
||
| var syncIterator = internalSlot.get(O, '[[SyncIteratorRecord]]')['[[Iterator]]']; | ||
| var returnMethod = syncIterator['return']; | ||
| if (returnMethod != null) { | ||
| if (!isCallable(returnMethod)) { | ||
| throw new $TypeError('iterator return is not a function'); | ||
| } | ||
|
|
||
| return resolve({ | ||
| value: value, | ||
| done: true | ||
| }); | ||
| } | ||
|
|
||
| var result = $apply(returnMethod, syncIterator, [value]); | ||
| if (!isObject(result)) { | ||
| throw new $TypeError('iterator return must return an object'); | ||
| } | ||
|
|
||
| return AsyncFromSyncIteratorContinuation(result, { | ||
| '[[Resolve]]': resolve, | ||
| '[[Reject]]': reject | ||
| }); | ||
| }); | ||
| } | ||
| }['return']); | ||
|
|
||
| CreateMethodProperty(AsyncFromSyncIteratorPrototype, 'throw', { | ||
| 'throw': function (value) { | ||
| var O = this; | ||
| return new Promise(function (resolve, reject) { | ||
| internalSlot.assert(O, '[[SyncIteratorRecord]]'); | ||
|
|
||
| var syncIterator = internalSlot.get(O, '[[SyncIteratorRecord]]')['[[Iterator]]']; | ||
| var throwMethod = syncIterator['return']; | ||
| if (throwMethod != null) { | ||
| if (!isCallable(throwMethod)) { | ||
| throw new $TypeError('iterator throw is not a function'); | ||
| } | ||
| throw value; | ||
| } | ||
|
|
||
| var result = $apply(throwMethod, syncIterator, [value]); | ||
| if (!isObject(result)) { | ||
| throw new $TypeError('iterator throw must return an object'); | ||
| } | ||
|
|
||
| return AsyncFromSyncIteratorContinuation(result, { | ||
| '[[Resolve]]': resolve, | ||
| '[[Reject]]': reject | ||
| }); | ||
| }); | ||
| } | ||
| }['throw']); | ||
|
|
||
| // eslint-disable-next-line consistent-return | ||
| return AsyncFromSyncIteratorPrototype; | ||
| }()); |
|
I think a potentially better option might be to implement a |
|
Yeah, you're probably right - a separate package just for AsyncIterator probably makes sense, especially given the iterator helpers proposal. I'll make these packages over the next few days and then update here. |
|
I have a work in progress implementation of the Iterator Helpers proposal in: @ExE‑Boss/Iterator‑Helpers. |
Needed for #68 and implementing iterator‑helpers.