monorepo: @react-rxjs/core @react-rxjs/utils @react-rxjs/dom

[josepot]

Remaining work:

• utils/groupInMap: JSDocs and tests
• utils/mergeWithKey: JSDocs and tests
• dom/batchUpdates: tests
• Basic README inside each package
• Make sure that the CI will keep working nicely after these changes

@voliva could you please help?

[codecov]

Codecov Report

Merging #58 into main will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #58   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           19        24    +5     
  Lines          246       289   +43     
  Branches        33        35    +2     
=========================================
+ Hits           246       289   +43     

Impacted Files	Coverage Δ
packages/core/src/SUSPENSE.ts	100.00% <ø> (ø)
packages/core/src/connectFactoryObservable.ts	100.00% <ø> (ø)
packages/core/src/connectObservable.ts	100.00% <ø> (ø)
packages/core/src/internal/COMPLETE.ts	100.00% <ø> (ø)
packages/core/src/internal/empty-value.ts	100.00% <ø> (ø)
packages/core/src/internal/noop.ts	100.00% <ø> (ø)
packages/core/src/internal/react-enhancer.ts	100.00% <ø> (ø)
packages/core/src/internal/share-latest.ts	100.00% <ø> (ø)
packages/core/src/internal/take-until-complete.ts	100.00% <ø> (ø)
packages/core/src/internal/useObservable.ts	100.00% <ø> (ø)
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af425e8...2c23c7b. Read the comment docs.

[josepot]

Ok, nice! It looks like codecov is able to pickup the code coverage of all the packages 🎉 Now, let's see if bundlesize can handle this as well

[voliva]

There was an attempt 😅 So much struggle to quickly describe what groupInMap and mergeWithKey does... It's the best I could do so far, I'll give it more thought later tonight.

I think I've fixed an issue with groupInMap, which was caused by continuousGroupBy: If the source completed, the completition wasn't propagated to the consumers.

But I also noticed something weird: When the source completes, groupInMap won't complete straight away, but will first delete every entry of the map one-by-one until it's empty, then complete. It makes sense the way it's coded, but I'm not sure it's an expected behaviour. We will try to figure this out.

[josepot]

Thanks a lot @voliva !

There was an attempt sweat_smile So much struggle to quickly describe what groupInMap and mergeWithKey does... It's the best I could do so far, I'll give it more thought later tonight.

LGTM

I think I've fixed an issue with groupInMap, which was caused by continuousGroupBy: If the source completed, the completition wasn't propagated to the consumers.

Nice! Thanks!

But I also noticed something weird: When the source completes, groupInMap won't complete straight away, but will first delete every entry of the map one-by-one until it's empty, then complete. It makes sense the way it's coded, but I'm not sure it's an expected behaviour. We will try to figure this out.

I think that I know a way to "fix" this, I will give it a shot. However, I wouldn't worry too much about it, because realistically it's very unlikely that the source actually completes... Meaning that I'm pretty sure that 99.99% of the times the source will be a top-level "subject" that won't complete.

[josepot]

@voliva

But I also noticed something weird: When the source completes, groupInMap won't complete straight away, but will first delete every entry of the map one-by-one until it's empty, then complete. It makes sense the way it's coded, but I'm not sure it's an expected behaviour. We will try to figure this out.

I ended up doing a proper fix for this that doesn't imply compromising the "internal" continuousGroupBy check it out. The reason why I did it like that is because I still hope that eventually the RxJS issue with groupBy will be addressed and then we will be able to use the official operator.

[josepot]

I'm going to go ahead and merge this. I will take care of the READMEs in a different PR.