-
Notifications
You must be signed in to change notification settings - Fork 6
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
Breaking Change: Support ESM #22
base: master
Are you sure you want to change the base?
Conversation
@@ -11,23 +11,41 @@ | |||
"asynciterator": "^3.9.0" | |||
}, | |||
"devDependencies": { | |||
"@types/node": "^22.1.0", |
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.
Note this devDependency is required to overcome the following error from AsyncIterator
.
This may mean that @types/node
or @types/event
needs to be added to the AsyncIterator
dependencies.
$ yarn run build
yarn run v1.22.22
$ npm run build:cjs && npm run build:esm
> asyncjoin@1.2.3 build:cjs
> tsc
join/HashJoin.ts:21:19 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
21 this.left.on('error', (error) => this.destroy(error));
~~
join/HashJoin.ts:21:32 - error TS7006: Parameter 'error' implicitly has an 'any' type.
21 this.left.on('error', (error) => this.destroy(error));
~~~~~
join/HashJoin.ts:22:20 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
22 this.right.on('error', (error) => this.destroy(error));
~~
join/HashJoin.ts:22:33 - error TS7006: Parameter 'error' implicitly has an 'any' type.
22 this.right.on('error', (error) => this.destroy(error));
~~~~~
join/HashJoin.ts:28:24 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
28 this.right.on('readable', () => this.readable = true);
~~
join/HashJoin.ts:29:24 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
29 this.right.on('end', () => { if (!this.hasResults()) this._end(); });
~~
join/HashJoin.ts:32:19 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
32 this.left.on('end', allowJoining);
~~
join/HashJoin.ts:34:14 - error TS2339: Property 'on' does not exist on type 'HashJoin<S, H, T>'.
34 this.on('newListener', (eventName) =>
~~
join/HashJoin.ts:34:33 - error TS7006: Parameter 'eventName' implicitly has an 'any' type.
34 this.on('newListener', (eventName) =>
~~~~~~~~~
join/HashJoin.ts:43:19 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
43 this.left.on('readable', () => this._addDataListenerIfNeeded());
~~
join/HashJoin.ts:110:19 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
110 this.left.on('data', addItem);
~~
join/SymmetricHashJoin.ts:16:14 - error TS2339: Property 'on' does not exist on type 'SymmetricHashJoin<S, H, T>'.
16 this.on('end', () => this._cleanup() );
~~
join/SymmetricHashJoin.ts:23:19 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
23 this.left.on('error', (error) => this.destroy(error));
~~
join/SymmetricHashJoin.ts:23:32 - error TS7006: Parameter 'error' implicitly has an 'any' type.
23 this.left.on('error', (error) => this.destroy(error));
~~~~~
join/SymmetricHashJoin.ts:24:20 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
24 this.right.on('error', (error) => this.destroy(error));
~~
join/SymmetricHashJoin.ts:24:33 - error TS7006: Parameter 'error' implicitly has an 'any' type.
24 this.right.on('error', (error) => this.destroy(error));
~~~~~
join/SymmetricHashJoin.ts:26:19 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
26 this.left.on('readable', () => this.readable = true);
~~
join/SymmetricHashJoin.ts:27:20 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
27 this.right.on('readable', () => this.readable = true);
~~
join/SymmetricHashJoin.ts:30:19 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
30 this.left.on ('end', () => { if (!this.hasResults()) this._end(); });
~~
join/SymmetricHashJoin.ts:31:20 - error TS2339: Property 'on' does not exist on type 'AsyncIterator<S>'.
31 this.right.on('end', () => { if (!this.hasResults()) this._end(); });
~~
Found 20 errors in 2 files.
Errors Files
11 join/HashJoin.ts:21
9 join/SymmetricHashJoin.ts:16
In fact I'm inclined to go a step further and suggest that this library be deprecated, and that the |
I agree in idea with all of the changes!
Given the invasiveness of this change, we'll need to have a close look at the impact in comunica performance-wise. Both base comunica and link traversal comunica make very heavy usage of the nested loop join (does not use dynamic nested loop join at all). So even the tiniest change in implementation could have a very big impact on them. So if you want to have a look at this impact, that would be great, and I can give you instructions. |
Sure, that also works :-) |
Breaking changes:
MergeIterator
which is the same asUnionIterator
inAsyncIterator
dist/
and built ESM files are inesm/
nestedLoopJoin
anddynamicNestedLoopJoin
are now functions that useAsyncIterator
methods rather than re-implementing existing logic in theAsyncIterator
library. These functions could be made even simpler and faster if someone wants to go to the effort of landing this multimap implementation in asynciterator. It may be worth considering the deletion of these two functions from theasyncjoin
library entirely.