- From: Domenic Denicola <notifications@github.com>
- Date: Mon, 31 Oct 2016 14:12:24 -0700
- To: whatwg/streams <streams@noreply.github.com>
- Message-ID: <whatwg/streams/pull/584/review/6526197@github.com>
domenic requested changes on this pull request. This doesn't seem quite right. I assume this is in reaction to https://github.com/whatwg/streams/pull/573#issuecomment-257418589 ? Can you help me understand that a bit more? For example if `O` is undefined then the current code will fail, right? So it's not every argument. Maybe what's just missing is the assert on `P`? > @@ -62,6 +62,10 @@ exports.IsFiniteNonNegativeNumber = v => { }; exports.InvokeOrNoop = (O, P, args) => { + assert(exports.typeIsObject(O) === true); This is not accurate. We use GetV, not Get. So in theory `O` could be anything. > @@ -62,6 +62,10 @@ exports.IsFiniteNonNegativeNumber = v => { }; exports.InvokeOrNoop = (O, P, args) => { + assert(exports.typeIsObject(O) === true); + assert(typeof P === 'string'); This is already in the spec, sort of, as "Assert: ! IsPropertyKey(P) is true." Probably there should be a (non-exported?) helper for that in this file, which checks if typeof is string or symbol. > @@ -62,6 +62,10 @@ exports.IsFiniteNonNegativeNumber = v => { }; exports.InvokeOrNoop = (O, P, args) => { + assert(exports.typeIsObject(O) === true); + assert(typeof P === 'string'); + assert(Array.isArray(args)); This is not accurate; the spec says "If args was not passed, let args be a new empty List." -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/whatwg/streams/pull/584#pullrequestreview-6526197
Received on Monday, 31 October 2016 21:12:55 UTC