Closed cfilipov closed 6 years ago
groue
commented
6 years ago That's great! Support for more reactive engines is a much welcomed contribution. It happens that I started my reactive life with RxSwift, because @fpillet is a very good evangelist. But I know there exist other engines out there. I'll be very happy to add cfilipov/ReactiveGRDB to the list of GRDB companion libraries when you think it is ready.
Tell me if you want any specific help.
cfilipov
commented
6 years ago I do have one specific area I would love help with if you can. I still haven't been able to get the testPrimaryKeyDiffScanner test to pass on my fork (all other tests pass). The test crashes with the following error:
Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'API violation - multiple calls made to -[XCTestExpectation fulfill] for .'This appears to be caused by calling fulfill multiple times in EventRecorder. However, when I run the same test on upstream RxGRDB, it also appears to be calling fulfill for every PrimaryKeyDiff, but no crash, and the test passes.
I placed a print call at the beginning of the on method of EventRecorder in both RxGRDB and ReactiveGRDB like so:
func on(_ event: Event<E>) {
print("Event: \(event)")
recordedEvents.append(event)
expectation.fulfill()
}See the output below. One thing to note is that my version is sending an extra initial event which is unexpected, but that still does not explain why calling fulfill multiple times crashes for my version and not for yours.
groue
commented
6 years ago Give me a few days, @cfilipov, I'll have a look.
cfilipov
commented
6 years ago Thank @groue! For what it's worth I've been using the ReactiveKit version for a few days now in non-trivial ways and have not seen any problems so far. But 100% test passed would definitely get more confidence.
Once the tests all pass I'll also have to go in and update the docs on my fork and comments to reflect the differences. So far it appears to be a very simple search & replace since none of the API changed except for a couple names (basically: "rx" -> "reactive", "Observable" -> "Signal" etc...).
groue
commented
6 years ago I've almost finished https://github.com/groue/GRDB.swift/pull/435, which introduces a new GRDB type: ValueObservation.
It aims at solving most value observation needs (including observation of requests that write: #42), as well as providing the base toolkit for reactive extensions. #46 has RxGRDB rely exclusively on ValueObservation for its value observables, and it looks that it works like charm.
I hope your ReactiveGRDB will be able to profit from it as well: I'll be interested in your feedback.
Now I'll be able to look at your scanner tests that fail!
groue
commented
6 years ago Now I'll be able to look at your scanner tests that fail!
I added a few .doOn(next:) calls in order to track the behavior of your observables. And I think that the difference comes from the different behavior of scan in RxSwift and in ReactiveKit. ReactiveKit's scan immediately starts outputting values, when RxSwift's scan waits until its base observable emits its initial value. ReactiveKit is not RxSwift ¯\_(ツ)_/¯.
groue
commented
6 years ago OK @cfilipov, I think I can close this issue. You'll have to do as I did with RxSwift, which is to think a little bit about how the database should interact with ReactiveKit, and how to give a useful API to users of cfilipov/ReactiveGRDB. Don't hesitate if you have more question!
In case you are interested in collaborating or taking a look, I made a quick and dirty port of RxGRDB for ReactiveKit.
Since the libraries aren't that different a lot of work was mechanical though some areas required more thought such as dealing with the explicit error type in ReactiveKit (I took the lazy way out for now and defined an
AnyErrortype). I got all unit tests to pass except fortestPrimaryKeyDiffScanner(still trying to figure out what's goin on with that one).