-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
Implement streams by adopting the reference implementation #3200
Draft
ninevra
wants to merge
64
commits into
jsdom:main
Choose a base branch
from
ninevra:streams-reference-subtree-move
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
64 commits
Select commit
Hold shift + click to select a range
3bfba25
Merge commit '0774e365e859ccffe8431441564ddd7bf202c0cf' as 'lib/jsdom…
ninevra 0774e36
Squashed 'lib/jsdom/living/streams/' content from commit 033c6d90
ninevra f58e402
Remove unneeded parts of streams repo
ninevra 7b5de92
Begin adapting to the outside-of-jsdom environment
ninevra 5e03c08
Register streams
ninevra 393edff
Rewrite imports
ninevra d067e8d
Use jsdom's AbortSignal
ninevra 1f92525
Enable streams web-platform-tests
ninevra 97e1dee
Globally disable jsshell tests
ninevra 95f8a3d
Pass through globalObject to abstract ops
ninevra 066a88c
Mark some tests failing
ninevra 8969474
Use wrappers for AbortSignal, DOMException
ninevra a107800
Always initialize _globalObject if necessary
ninevra e544546
Save globalObject in InitializeReadableStream
ninevra 942da52
Save globalObject in InitializeWritableStream
ninevra 1093ce0
Save globalObject in SetUpReadableByteStreamController
ninevra 6f5f24b
Use Uint8Arrays and ArrayBuffers from the globalObject
ninevra 554b1ac
Mark transferable tests as fail-slow for now
ninevra eafe528
Mark outdated tests failing for now
ninevra 476e47a
Mark remaining failing tests
ninevra 0f06828
Fix lint issues
ninevra bec5fd1
Don't fake transfering buffers
ninevra f3ac927
Flatten streams directory hierarchy
ninevra 4073fbd
Revert "Rewrite imports"
ninevra 544c638
Reregister streams
ninevra c4a2865
Squashed 'lib/jsdom/living/streams/' changes from 033c6d90..8a7d92b5
ninevra 5c22cd2
Merge commit 'c4a2865a2032c2ee4dc1767b8c769c7482bf531f' into streams-…
ninevra b1f75bd
Update web-platform-tests
ninevra 3607e6e
(Partially) update to-run for new web-platform-tests
ninevra 35588a0
Update to-run for streams web-platform-tests
ninevra 16fa7d4
Cite the vm bug causing patched-global failures
ninevra 3a64fc0
Remove unused code
ninevra f9f8c73
Remove reference implementation's licensing
ninevra e97f8c3
Revert "Update web-platform-tests"
ninevra 7d20c24
Revert "(Partially) update to-run for new web-platform-tests"
ninevra 60569b1
Revert "Update to-run for streams web-platform-tests"
ninevra 26ad931
Mark an incidentally fixed xhr test!
ninevra 2b3849c
Begin fixing style
ninevra 15fcd00
Change capitalization of abstract operations
ninevra cd7f18f
Capitalize ViewConstructor
ninevra 8e3a327
Fix function style
ninevra fba4fb5
Fix line length
ninevra ac6b8b9
Merge branch 'master' into streams-reference-subtree-move
ninevra 99864ae
Revert "Revert "Update to-run for streams web-platform-tests""
ninevra 4243cb4
Mark a test dependent on free function postMessage
ninevra 6901cf3
Disable readable byte streams
ninevra 6afefd0
Remove manual AbortSignal typechecking
ninevra f2f4c67
Use jsdom's mixin util
ninevra 8625950
Remove uponFulfillment
ninevra 68b3b87
Remove uponRejection
ninevra 8dfadd7
Remove transformPromiseWith
ninevra f47c653
Remove uponPromise
ninevra 7588448
Remove performPromiseThen
ninevra c73a20e
Remove waitForAllPromise
ninevra beaac8f
Remove the promise sidetable
ninevra 38f2007
Remove assertions
ninevra 4ce3283
Rename webidl.js to promises.js
ninevra 3e2cb56
Reenable tests blocked by nodejs/node#38918
ninevra 5f574da
Predefine globals in the VM to work around the bug
ninevra 0104cb5
Fix bug in readableStreamTee
ninevra b84ebcb
Fix lint errors
ninevra 0bfe33f
Predefine global properties as non-enumerable
ninevra 3f05836
Use this instead of globalThis for node 10 support
ninevra c254c7b
Remove unneed support for resetQueue asserts
ninevra File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
27 changes: 27 additions & 0 deletions
27
lib/jsdom/living/streams/ByteLengthQueuingStrategy-impl.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
"use strict"; | ||
|
||
const sizeFunctionWeakMap = new WeakMap(); | ||
|
||
exports.implementation = class ByteLengthQueuingStrategyImpl { | ||
constructor(globalObject, [{ highWaterMark }]) { | ||
this._globalObject = globalObject; | ||
this.highWaterMark = highWaterMark; | ||
} | ||
|
||
get size() { | ||
initializeSizeFunction(this._globalObject); | ||
return sizeFunctionWeakMap.get(this._globalObject); | ||
} | ||
}; | ||
|
||
function initializeSizeFunction(globalObject) { | ||
if (sizeFunctionWeakMap.has(globalObject)) { | ||
return; | ||
} | ||
|
||
// We need to set the 'name' property: | ||
// eslint-disable-next-line prefer-arrow-callback | ||
sizeFunctionWeakMap.set(globalObject, function size(chunk) { | ||
return chunk.byteLength; | ||
}); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
[Exposed=(Window,Worker,Worklet)] | ||
interface ByteLengthQueuingStrategy { | ||
constructor(QueuingStrategyInit init); | ||
|
||
readonly attribute unrestricted double highWaterMark; | ||
readonly attribute Function size; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
"use strict"; | ||
|
||
const sizeFunctionWeakMap = new WeakMap(); | ||
|
||
exports.implementation = class CountQueuingStrategyImpl { | ||
constructor(globalObject, [{ highWaterMark }]) { | ||
this._globalObject = globalObject; | ||
this.highWaterMark = highWaterMark; | ||
} | ||
|
||
get size() { | ||
initializeSizeFunction(this._globalObject); | ||
return sizeFunctionWeakMap.get(this._globalObject); | ||
} | ||
}; | ||
|
||
function initializeSizeFunction(globalObject) { | ||
if (sizeFunctionWeakMap.has(globalObject)) { | ||
return; | ||
} | ||
|
||
// We need to set the 'name' property: | ||
// eslint-disable-next-line prefer-arrow-callback | ||
sizeFunctionWeakMap.set(globalObject, function size() { | ||
return 1; | ||
}); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
[Exposed=(Window,Worker,Worklet)] | ||
interface CountQueuingStrategy { | ||
constructor(QueuingStrategyInit init); | ||
|
||
readonly attribute unrestricted double highWaterMark; | ||
readonly attribute Function size; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
dictionary QueuingStrategy { | ||
unrestricted double highWaterMark; | ||
QueuingStrategySize size; | ||
}; | ||
|
||
callback QueuingStrategySize = unrestricted double (optional any chunk); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
dictionary QueuingStrategyInit { | ||
required unrestricted double highWaterMark; | ||
}; |
123 changes: 123 additions & 0 deletions
123
lib/jsdom/living/streams/ReadableByteStreamController-impl.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
"use strict"; | ||
const { CancelSteps, PullSteps } = require("./abstract-ops/internal-methods.js"); | ||
const { resetQueue } = require("./abstract-ops/queue-with-sizes.js"); | ||
const aos = require("./abstract-ops/readable-streams.js"); | ||
|
||
const ReadableStreamBYOBRequest = require("../generated/ReadableStreamBYOBRequest.js"); | ||
|
||
exports.implementation = class ReadableByteStreamControllerImpl { | ||
get byobRequest() { | ||
if (this._byobRequest === null && this._pendingPullIntos.length > 0) { | ||
const firstDescriptor = this._pendingPullIntos[0]; | ||
const view = new this._globalObject.Uint8Array( | ||
firstDescriptor.buffer, | ||
firstDescriptor.byteOffset + firstDescriptor.bytesFilled, | ||
firstDescriptor.byteLength - firstDescriptor.bytesFilled | ||
); | ||
|
||
const byobRequest = ReadableStreamBYOBRequest.new(this._globalObject); | ||
byobRequest._controller = this; | ||
byobRequest._view = view; | ||
this._byobRequest = byobRequest; | ||
} | ||
|
||
return this._byobRequest; | ||
} | ||
|
||
get desiredSize() { | ||
return aos.readableByteStreamControllerGetDesiredSize(this); | ||
} | ||
|
||
close() { | ||
if (this._closeRequested === true) { | ||
throw new TypeError("The stream has already been closed; do not close it again!"); | ||
} | ||
|
||
const state = this._stream._state; | ||
if (state !== "readable") { | ||
throw new TypeError(`The stream (in ${state} state) is not in the readable state and cannot be closed`); | ||
} | ||
|
||
aos.readableByteStreamControllerClose(this); | ||
} | ||
|
||
enqueue(chunk) { | ||
if (chunk.byteLength === 0) { | ||
throw new TypeError("chunk must have non-zero byteLength"); | ||
} | ||
if (chunk.buffer.byteLength === 0) { | ||
throw new TypeError("chunk's buffer must have non-zero byteLength"); | ||
} | ||
|
||
if (this._closeRequested === true) { | ||
throw new TypeError("stream is closed or draining"); | ||
} | ||
|
||
const state = this._stream._state; | ||
if (state !== "readable") { | ||
throw new TypeError(`The stream (in ${state} state) is not in the readable state and cannot be enqueued to`); | ||
} | ||
|
||
aos.readableByteStreamControllerEnqueue(this._globalObject, this, chunk); | ||
} | ||
|
||
error(e) { | ||
aos.readableByteStreamControllerError(this, e); | ||
} | ||
|
||
[CancelSteps](reason) { | ||
aos.readableByteStreamControllerClearPendingPullIntos(this); | ||
|
||
resetQueue(this); | ||
|
||
const result = this._cancelAlgorithm(reason); | ||
aos.readableByteStreamControllerClearAlgorithms(this); | ||
return result; | ||
} | ||
|
||
[PullSteps](readRequest) { | ||
const stream = this._stream; | ||
// Assert: aos.readableStreamHasDefaultReader(stream) === true | ||
|
||
if (this._queueTotalSize > 0) { | ||
// Assert: aos.readableStreamGetNumReadRequests(stream) === 0 | ||
|
||
const entry = this._queue.shift(); | ||
this._queueTotalSize -= entry.byteLength; | ||
|
||
aos.readableByteStreamControllerHandleQueueDrain(this); | ||
|
||
const view = new this._globalObject.Uint8Array(entry.buffer, entry.byteOffset, entry.byteLength); | ||
|
||
readRequest.chunkSteps(view); | ||
return; | ||
} | ||
|
||
const autoAllocateChunkSize = this._autoAllocateChunkSize; | ||
if (autoAllocateChunkSize !== undefined) { | ||
let buffer; | ||
try { | ||
buffer = new this._globalObject.ArrayBuffer(autoAllocateChunkSize); | ||
} catch (bufferE) { | ||
readRequest.errorSteps(bufferE); | ||
return; | ||
} | ||
|
||
const pullIntoDescriptor = { | ||
buffer, | ||
bufferByteLength: autoAllocateChunkSize, | ||
byteOffset: 0, | ||
byteLength: autoAllocateChunkSize, | ||
bytesFilled: 0, | ||
elementSize: 1, | ||
ViewConstructor: this._globalObject.Uint8Array, | ||
readerType: "default" | ||
}; | ||
|
||
this._pendingPullIntos.push(pullIntoDescriptor); | ||
} | ||
|
||
aos.readableStreamAddReadRequest(stream, readRequest); | ||
aos.readableByteStreamControllerCallPullIfNeeded(this); | ||
} | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
[Exposed=(Window,Worker,Worklet)] | ||
interface ReadableByteStreamController { | ||
readonly attribute ReadableStreamBYOBRequest? byobRequest; | ||
readonly attribute unrestricted double? desiredSize; | ||
|
||
void close(); | ||
void enqueue(ArrayBufferView chunk); | ||
void error(optional any e); | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW you can do
The
size
function's name property will be automatically set.Also, do we need to create the function in
globalObject
, to ensure the prototype is correct?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is straight from the reference impl. Personally I have a mild preference for
function size() {}
overconst size =
, I think it makes what's going on a little more clear.I think you're right about the prototype, but that's a jsdom-wide problem, isn't it? E.g.
Object.getPrototypeOf(window.Headers) !== window.Function.prototype
. Not sure it's worth hacking around that issue here specifically, unless there's a really simple fix.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using
function size() { }
is important because per spec the result has a.prototype
property, whereas arrow functions do not. (I wonder if that's tested.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ninevra
Ah yes, indeed.
@domenic Printing
gives undefined in Chrome and Firefox. The Streams spec calls CreateBuiltinFunction without calling MakeConstructor, which is the abstract op that adds the
prototype
property. So it would appear that we should in fact use arrow functions after allThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patch submitted upstream