Fix plugin dependsOn always throwing error

[StevenJNPearce]

Ran into this whilst getting familiar with the project to work on #106 const serviceImplNames = Object.keys(_.filter(serviceDefinitions, 'implementation')); This is returning [ '0', '1', '2', ... ], so ERR_MSG_MISSING_SERVICES is thrown even when a correct service name is provided under dependsOn in the plugin config.

[CLAassistant]

All committers have signed the CLA.

[raulk]

@StevenJNPearce Hey! Thanks for pointing this out. However, I'm not convinced we have a bug here. The error is supposed to be thrown if the plugin requires a service, but no implementation has been loaded for that service.

Could you maybe cook up a test case that leads to this?

[StevenJNPearce]

@raulk No problem, I've written a test and reverted the change in bootstrap.ts so you can see it failing.

[StevenJNPearce]

@raulk The issue here appears to be a quirk with the behavior of lodash's filter method. It is stripping the the keys of the input object in the returned array. After reading through the lodash docs, I've replaced the .filter with .pickBy - the pickBy method is intended for filtering on objects rather than collections. Tests are now passing.

[StevenJNPearce]

I've found a second bug here in addition to the one documented above. I've commented the code below to explain.

  const missingServices = plugins
    .filter(plugin => plugin.dependsOn && plugin.dependsOn.services)
    .map(({ name, dependsOn }: EthqlPlugin) => ({
      name,
      missing: dependsOn.services.filter(s => !serviceImplNames.includes(s)), // Returns an empty array when nothing matches the predicate i.e. there are no missing services.
    }))
    .filter(({ missing }) => missing); // here the empty array evaluates to true, because [] == false 

You probably read that last line and thought it doesn't make any sense. You're right. Anyway, fixed by changing to .filter(({ missing }) => missing.length) I've also updated the test to catch this case.

[raulk]

👍 I see what's going on. I implemented these checks, but didn't make erc20 depend on web3, decoder and ethService. End result is that this code was not being executed in tests nor in production.

Thanks for catching it. On the way, I also found a change in behaviour in ts-node 7.0.0 that breaks declaration merging, so I will be pushing a fix for that. More info here: https://github.com/TypeStrong/ts-node#help-my-types-are-missing

I'll also add the dependencies in the erc20 plugin.