See https://github.com/microsoft/tslib/pull/193
fix: add more typescript leaks
fix: rename the private closed
property to _closed
Node.js 18 introduced a Readable.closed
property which is read-only - this removes the accidental overlap.
OK, updated #111 with the code. The tests do pass... https://github.com/dominykas/rethinkdb-ts/actions/runs/3900771040
fix: rename the private closed
property to _closed
Node.js 18 introduced a Readable.closed
property which is read-only - this removes the accidental overlap.
revert: undo cursor.ts
changes introduced in 5cb4f4cddf6d69f575b9ae6ceba3751071caa9ae and 52416537557f4da04aad1e6790885c8a5064dcab
While these changes solve the Readable.closed
property overlap, they introduce a race condition where the end
event may be emitted before all the data
events complete, resulting in incomplete results when using pipe()
.
fork
test: reproduce pipe() problems
ci: enable Node.js 18 in CI
fix: rename a private property which collides with a new one introduced in Node.js 18
Merge pull request #1 from dominykas/fix-cursor-node18
rethinkdb-ts@2.4.22
(because 2.4.23
is the first broken version)ReadableStream.closed
was introduced in Node.js 18, we need to rename the private Cursor.closed
to avoid the naming collision.fix: rename a private property which collides with a new one introduced in Node.js 18
ci: enable Node.js 18 in CI
ci: enable Node.js 18 in CI
ci: enable Node.js 18 in CI
ci: enable Node.js 18 in CI
ci: enable Node.js 18 in CI
Reason for the change
For now, this just adds a reproduction test-case for https://github.com/rethinkdb/rethinkdb-ts/issues/108
Checklist
Reason for the change
For now, this just adds a reproduction test-case for https://github.com/rethinkdb/rethinkdb-ts/issues/108
Checklist
(closing - will reopen from a different branch, as I'd like to use my main
for forking this).
Ah, I didn't realize that Readable.closed
was only introduced in Node.js 18 - so it seems that naming collision was definitely unintended and the Cursor.closed
was not being used for Stream
purposes in Node.js, so my guess is that it's safe to simply rename?
I'm happy to open a PR with the change to revert cursor.ts
changes and rename the closed
to _closed
if that's acceptable?
Just testing things out, I reverted the cursor.ts
changes from https://github.com/rethinkdb/rethinkdb-ts/commit/5cb4f4cddf6d69f575b9ae6ceba3751071caa9ae and https://github.com/rethinkdb/rethinkdb-ts/commit/52416537557f4da04aad1e6790885c8a5064dcab and then simply renamed the private closed
to private _closed
so that it does not collide with the one inheritted from the Stream
and all tests (incl. my repro test case) seem to pass on Node.js 14, 16 and 18.
I don't know if this is the right thing to do, though, but my hunch is that Cursor.closed
might have never been intended to override the Readable.closed
? Otherwise why declare it as a private
at all?
Yes, the test I have in #109 is also failing on node@14.
Reason for the change
For now, this just adds a reproduction test-case for https://github.com/rethinkdb/rethinkdb-ts/issues/108
Checklist