EOSIO / demux-js

💫 Deterministic event-sourced state and side effect handling for blockchain applications
MIT License
307 stars 71 forks source link

BaseActionWatcher should require an IActionReader instead of an AbstractActionReader #165

Closed olivierbeaulieu closed 5 years ago

olivierbeaulieu commented 5 years ago

I am in the process of writing an AbstractActionReader to connect to the dfuse.io GraphQL API rather than directly to a Nodeos via demux-eos.

I'm running into a few issues. Since dfuse navigates forks for us, and the way rollbacks are handled is different, I don't need most of the private methods in AbstractActionReader. The blockHistory is unnecessary to solve this problem. So ideally, I'd prefer to use implements AbstractActionReader rather than extends AbstractActionReader.

However BaseActionWatcher requires an AbstractActionReader, which is a bit problematic in Typescript. In TS, you need to have all private methods and fields defined to match, and even then the constructors won't match so it won't compile.

A solution would be to have BaseActionReader require something that implements IActionReader rather than an instance of AbstractActionReader.

Let me know if this makes sense, and I can proceed to the implementation right away.

flux627 commented 5 years ago

This is reasonable- I'll have a PR ready on Monday!

olivierbeaulieu commented 5 years ago

Thanks for the response! I already created #168 to address this.

flux627 commented 5 years ago

Check out the PR referenced above, I went a bit further and cleaned up the interface a bit. Let me know what you think.