This module needs to go away

[achingbrain]

This module was create to break the dependency cycle between non-trivial libp2p components (e.g. ones that need access to other libp2p components) and libp2p itself (the holder of those components).

We also split each interface out of @libp2p/interfaces into it's own package in order to not have to update every module when a breaking change was released in a component interface that a given module did not consume.

The problem is, of course, that all those interfaces are depended on by this module, meaning this module needs a major whenever a breaking change is released in a component interface, so every module that uses this module then needs updating whether it uses the component with the breaking change or not. Bleh.

[abetaev]

i was looking into js-libp2p-kad-dht when discovered this.

i think dependency injection would look nice here.

class RoutingTable {
// ...
  init(di: Components) {
    di.put("routing-table", this)
    this.peerId = di.get("peerId")
    this.metrics = di.get("metrics")
  }
}
// ...
}

dependencies resolved lazily when used, the injected values are proxies; di container may provide more convenience features like logging/tracing, metrics, etc...

pros:

• this module becomes an utility and does not depend on interfaces
• simplifies dependency management between services
• exposes dependencies explicitly (increases code readability)

cons:

• it's less typesafe (but it's the only way i see to remove dependency on interfaces)
• may create some obstacles for testing
• may add some overhead (mostly depends on implementation)

there might be a js library which already does all this, i cannot recommend anything particular. such library may replace this module.

[achingbrain]

I wonder if we couldn't just destructure the deps we need:

export interface Init {
  peerId: PeerId
  metrics: Metrics
}

class RoutingTable {
// ...
  init(init: Init) {
	const { peerId, metrics } = init
    this.peerId = peerId
    this.metrics = metrics
  }
}
// ...
}

libp2p would pass a blob of components in as before, and it might need to be careful about the order things are created but at least each component would only depend on it's dependencies and this module could be retired.

[MarcoPolo]

Quickly commenting on @abetaev, losing type information would be a major downside and not worth it. It also sounds like you're simply using a map here.

---

@achingbrain yes. This is the caller defining their interface. One of the best parts of typescript is that it enables this. We don't have to import the Components type. We just say we are willing to accept whatever as long as it fulfills this interface (e.g. Init in your example above).

Let's get rid of this module and do the above pattern.

[achingbrain]

Getters can be added to this module which would let us roll the above pattern out module by module, no need for a big bang then.

[achingbrain]

Having said that there are a few caveats.

Let's declare this generic interface, letting modules define the dependencies they need:

export interface Initializable<Components = unknown> {
  init: (components: Components) => void
}

I have a component:

interface MyDependencies {
  foo: Foo
  bar: Bar
}

class MyComponent implements Initializable<MyDependencies> {
  private foo? Foo
  private bar? Bar

  init (components: MyDependencies): void {
    this.foo = components.foo
    this.bar = components.bar
  }

  someTimeLater () {
    this.foo!.doSomething()
  }
}

Notable things:

1. Because the fields are not set in the constructor they need the optional property modifier ? and cannot be marked readonly
2. Because the fields are now optional we need to null-guard on their use or use the non-null assertion operator when we access them

The non-null assertion operator is just a flag to the compiler, it's removed from the compiled js so there is no runtime penalty, but neither is there any type safety - if I forgot to set this.foo in init, the compiler would wave it through and it would fail at runtime.

The only way round this that I can see is to use constructor injection, but that would mean we need to switch up everything to use factory functions rather than constructors, for example if I had a transport that took some constructor arg config:

export class MyTransport implements Initializable<MyTransportDependencies> {
  constructor (init: MyTransportInit) {
    // ...
  }

  init (deps: MyTransportDependencies) {
    // ...
  }
}

we currently instantiate instances of transports and pass them to the libp2p factory function:

const node = createLibp2p({
  transports: [
    new MyTransport({
      // transport-specific options
    })
  ]
  // ...
})

Instead we'd need to change the implementation to use some sort of factory function (a factory factory, if you will):

class MyTransport {
  constructor (deps: MyTransportDependencies, init: MyTransportInit) {
    // ...
  }
}

export function myTransport (init: MyTransportInit) {
  return (deps: MyTransportDependencies) => {
    return new MyTransport(deps, init)
  }
}

Then invoke the factory when creating the node:

const node = createLibp2p({
  transports: [
     myTransport({
      // transport-specific options
    })
  ]
  // ...
})

Internally libp2p would then call the factory with the components blob.

This gives us type safety back though is a more disruptive change.