Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fx.Evaluate: Support dynamic graphs by returning fx.Option from constructors #1132

Open
abhinav opened this issue Nov 3, 2023 · 0 comments
Assignees

Comments

@abhinav
Copy link
Collaborator

abhinav commented Nov 3, 2023

Feature request

Is your feature request related to a problem? Please describe.

This scenario was previously described in #1131. It's reproduced below to save a click.

Suppose a program has a Store interface with two implementations: a memory-backed version and a Redis-backed version.

type Store interface {
	Get(k string) (v string, error)
	Set(k, v string) error
}

type MemoryStore struct{ m map[string]string }

func NewMemoryStore() *MemoryStore

type RedisStore struct{ /* ... */ }

func NewRedisStore(*redis.Client) *RedisStore

The memory store has no dependencies, while the Redis store depends on a Redis client.

flowchart LR
  MemoryStore
  RedisStore -->|NewRedisStore| RedisClient[*redis.Client]
Loading

The application wants to pick between the two implementations based on a configuration parameter.
Imagine:

func NewStore(cfg Config, memStore *MemoryStore, rediStore *RedisStore) Store {
	if cfg.Development {
		return memStore
	}
	return redisStore
}

However, as a result of this, NewStore has a hard transitive dependency on the Redis client, even if the Redis store won't be used.

flowchart LR
  MemoryStore
  RedisStore -->|NewRedisStore| RedisClient[*redis.Client]
  Store -->|NewStore| Config & MemoryStore & RedisStore
  Store -.-> RedisClient
Loading

Ideally, NewStore should be able to specify which dependency set it wants to use after looking at the configuration.

Describe the solution you'd like

I'd like to propose adding a new intrinsic operation to the Fx model to accompany the existing intrinsics: Provide, Invoke, and Decorate.

package fx

func Evaluate(...any) fx.Option

The new operation, tentatively named fx.Evaluate, will represent an unevaluated part of the graph.
image

Functions passed to fx.Evaluate will always be evaluated. They will feed new information to the graph in the form of fx.Option return values.
These new operations may form connections that previously did not exist.
image

With this operation, the scenario described above would be solved like so:

func NewStore(cfg Config) fx.Option {
    if cfg.Development {
        return fx.Provide(func(s *MemoryStore) Store { return s })
    }
    return fx.Provide(func(s *RedisStore) Store { return s })
}

// Or alternatively:

func NewStore(cfg Config) fx.Option {
    if cfg.Development {
        return fx.Provide(
            fx.Annotate(NewMemoryStore, fx.As(new(Store))),
        )
    }
    return fx.Provide(
        fx.Annotate(NewRedisStore, fx.As(new(Store))),
    )
}

// In main:

fx.New(
    // ...
    fx.Evaluate(NewStore),
)

Describe alternatives you've considered

fx.Replace was considered but is insufficient for this functionality.
It cannot conditionally severe the dependency between NewStore and *redis.Client.
See this comment.

Is this a breaking change?
No, this is not a breaking change.

Additional context
This functionality has been brought up before and has been referred to with names such as: monadic graph, dynamic provides, late provides.

There was a prior attempt to implement this in #699 (tracked in #698). This attempt was rejected because of the following reasons per this comment.

  • There was risk of UX confusion by overloading fx.Provide. This is not an issue if we introduce a new intrinsic.
  • The internals of the library were actively being reworked for fx.Decorate. This is done now.
  • There were concerns about how "late provides" would interact with lifecycles. Again, this is not an issue with a new intrinsic because new expectations can be established: Evaluate always runs, so lifecycle events there always run.

Fx startup roughly takes the following shape:

provides, decorates, invokes = [...], [...], [...]

provideAll(provides)
decorateAll(decorates)
invokeAll(invokes)

start()

With the new intrinsic, this could effectively become:

provides, decorates, invokes, evaluates = [...], [...], [...], [...]

provideAll(provides)
decorateAll(decorates)

while len(evaluates) > 0:
  e = pop(evaluates)
  ps, ds, is, es = evaluate(e)
  provideAll(ps)
  decorateAll(ds)
  invokes.append(is...)
  evaluates.append(es...)

invokeAll(invokes)
@sywhang sywhang self-assigned this Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants