uber-go/fx

Specify at least weak order for values in the value group

oakad opened this issue · 3 comments

oakad commented

The documentation on the value groups presently says: "This will execute all constructors that provide a value to that group in an unspecified order".

I would like to propose a change to the tune: "The constructors will be executed in the topological order of their inter-dependencies and the value group will be populated accordingly" or something like that. That is, if constructor B executes after constructor A in the dependency graph, then pos(B) > pos(A) will hold true by contract. In cases where constructors can execute in parallel, the respective order is not important (weak).

I suspect it to be the case already (after all, it's a natural thing to do), but it will be real nice to codify this.

Why is this needed?

Occasionally, the dependency between object exists not at construction time, but at some later ("registration") time. For example, we can have an http service implemented like so:

  makeHandlerA() httpHandler
  makeHandlerB() httpHandler
  makeHandlerC() httpHandler
  makeHandlerD() httpHandler
  newHttpService(handlers []httpHandler)

While not directly dependent on each other, the nature of the service may require handler A to be registered first, and handler D to be registered last. We can easily express this in the construction tree by using "dummy" named parameters or possibly, "tag" types:

  makeHandlerA() httpHandler
  makeHandlerB(handlerA) httpHandler
  makeHandlerC(handlerA) httpHandler
  makeHandlerD(handlerB, handlerC) httpHandler
  newHttpService(handlers []httpHandler)

But, for this to really work we need "handlers" value group to be ordered in the same order, either [A, B, C, D] or [A, C, B, D], not something "unspecified".

Hey @oakad .

We specifically randomize the order of the values in value groups on purpose.

#886 captures the rationale behind this design choice, as well as how you can enforce ordering in use cases such as yours.

Hope that helps.

oakad commented

It's totally not the same use case.

To my opinion, the forced breaking of ordering is a major design mistake (same applies to go hash map randomization, btw). Basically amounts to "let's increase entropy for no reason and make people jump through hoops to undo the mess". :-)

Sure, we can agree to disagree - anyone can have varying opinions about design philosophies of programming languages and library/frameworks. You may believe that this is an unnecessary entropy that makes you jump through hoops. In others views such as mine, deliberately shuffling the order of value groups, similar to go hash map iteration orders, adheres to the design choices that enforce programmers to not take a dependency on undocumented behavior.

The behavior of value groups is implemented and documented precisely the way it was designed - Value groups were designed to be an unordered set of types when we designed the API. The discussion I linked above explains pretty clearly why that design decision was made.