Expose operation counts from Persistence layer (take 2) #2165
Conversation
schmidt-sebastian
force-pushed
the
mrschmidt/indexfree-counter2
branch
from
f023559 to
eea6982
Compare
|
Note that I did experiment with https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy, but was never able to get it to work via TS Node. It would also require us to use a different tsconfig for the unit tests (to add a dependency on es2015.proxy). |
|
"Coverage decreased (-0.05%) to 89.621%" I don't believe you. |
schmidt-sebastian
force-pushed
the
mrschmidt/indexfree-counter2
branch
from
0005f4a to
6966e26
Compare
There was a problem hiding this comment.
I like this better. I'm not 100% sure on the cost-benefit ratio given this is still 270 lines of code and it's going to be fragile in the face of any changes to the RemoteDocumentCache, MutationQueue, or QueryEngine interfaces. But it's well isolated so if it ends up being more trouble than it's worth, it'll be easy to rework. So in that respect, I like it a lot. 😄
| private indexManager: IndexManager | ||
| readonly remoteDocumentCache: RemoteDocumentCache, | ||
| readonly mutationQueue: MutationQueue, | ||
| readonly indexManager: IndexManager |
There was a problem hiding this comment.
I'm guessing this is why coverage decreased. Making these readonly probably costs us a few bytes. 😄
| * A test-only query engine that forwards all API calls and exposes the number | ||
| * of documents and mutations read. | ||
| */ | ||
| export class ForwardingCountingQueryEngine implements QueryEngine { |
There was a problem hiding this comment.
ForwardingCountingQueryEngine doesn't read super nicely. I think I would do OpCountingQueryEngine or OpCountingQueryEngineWrapper or something.
There was a problem hiding this comment.
I dropped the "Forwarding" part. Not sold on your suggestions :)
| } | ||
|
|
||
| private wrapRemoteDocumentCache( | ||
| target: RemoteDocumentCache |
There was a problem hiding this comment.
"target" is already used for something else in the client. Find a new term. 😄 E.g.: remoteDocumentCache, internal, wrapped
There was a problem hiding this comment.
I used "source" (which is also used, but much more lightly...)
|
Merging this since the pervious CI run passed and changes are just renames/minor. |
"Sebastian uses his time wisely."
This is another take on #2164.
It solves the following issues: