-
Notifications
You must be signed in to change notification settings - Fork 45
Add compound plugins #281
base: master
Are you sure you want to change the base?
Add compound plugins #281
Conversation
This is an interesting idea... but I would like to try and keep the API surface area of fusion as small as possible. This could be done with the existing api using const ArrayToken = createToken('TestToken');
const plugin = createPlugin({
deps: {arr: ArrayToken},
provides: ({arr}) => {
//arr = [1, 2, 3]
},
});
app.register(ArrayToken, [1]);
app.enhance(ArrayToken, (prev) => [...prev, 2]);
app.enhance(ArrayToken, (prev) => [...prev, 3]);
app.register(plugin); |
I agree the surface area should be as minimal as possible. This only adds a new method you don’t have to use. The surface area of the FusionApp is unchanged. More generally, this feature provides an easy way to implement patterns like composite or strategy with full IOC. For example, the cleanest way to implement the strategy pattern would be to not have any direct coupling between strategies and context, but only rely on a common interface for strategies. For the GraphQL resolver case we implemented a solution somewhat like this:
However, this is messy as you put knowledge into the resolvers about how they are consumed, which is unrelated to the function they actually fulfil. You can wrap the above in a utility function (which we do) but we believe the pattern is general enough (we already have other uses for it in our project) that Fusion should support it directly. |
Chatted with Rasmus offline about this problem and he brings up lots of good points. I think we should consider various options for handling plugins that need to compose with each other. The current approach we have been using is to handle composition in user code. For example, app.register(TokenA, ThingA)
app.register(TokenB, ThingB)
app.register(ActualToken, createPlugin({
deps: { a: TokenA, b: TokenB },
provides: ({a, b}) => compose(a, b)
})) While a bit verbose, this generally works ok. However it has a few downsides. This works well if you are grouping your plugins by type, (i.e. all redux related things colocated) but falls apart a bit when grouping by feature (for example, if you want to put My main problem with the current approach with the
One potential option is to support import compose from 'just-compose'
app.compose(Token, A);
app.compose(Token, B);
app.register(Token.compose, (items) => compose(items))
// or similarly
const TokenA = createComposeToken('A', compose); This is a brief look at one option which leaves out many details such as how this interacts with |
I'm generally against the idea of adding special semantics to do things that can be accomplished with core language features. If you want each value to be overridable, that's the only time you'd need to create tokens for them. const plugin = createPlugin({
deps: {a: A, b: B, c: C},
provides: ({a, b, c}) => {
const arr = [a, b, c] // 1, 2, 3
// ...
},
});
app.register(A,
createPlugin({
provides: () => 1,
})
);
app.register(B, 2);
app.register(C, 3);
app.register(plugin); If you want one-off values, you can either hard-code them or use a factory pattern // hard-coded (in provider)
const plugin = createPlugin({
deps: {a: A},
provides: ({a}) => {
// a = [1, 2, 3]
},
});
app.register(A,
createPlugin({
provides: () => [1, 2, 3],
})
);
app.register(plugin);
// hard-coded (in consumer)
const plugin = createPlugin({
deps: {a: A},
provides: ({a}) => {
const arr = [a, 2, 3]
},
});
app.register(A,
createPlugin({
provides: () => 1,
})
);
app.register(plugin);
// factory (encapsulating provider)
const plugin = createPlugin({
deps: {a: A},
provides: ({a}) => {
// a = [1, 2, 3]
},
});
const a = (b, c) => createPlugin({
provides: () => [1, b, c],
});
app.register(A, a(2, 3));
app.register(plugin);
// factory (encapsulating consumer)
const plugin = (b, c) => createPlugin({
deps: {a: A},
provides: ({a}) => {
const arr = [a, b, c] // 1, 2, 3
},
});
const a = createPlugin({
provides: () => 1,
});
app.register(A, a);
app.register(plugin(2, 3)); Making a special Another issue is that I'm not sure how one can override the token for testing if calling app.register pushes to a list instead of overriding. One possibility would be to use enhance, but that seems like abusive API usage. We already have cases where token registration reads from a factory (e.g. |
The factory pattern described in @lhorie example does not solve the problem we are facing. The compound tokens might be niche, but I still think they have merit. Our app is rather modularised, meaning that it contains several parts which are somewhat pluggable. Therefore having to register a modules parts several places will make it the separation very brittle. |
Circling back on this as uses for it keep popping up in our project. To reiterate, the general pattern we want to support is where one “delegator” plugin provides a service via multiple "handler" plugins that implement a common interface. Currently it does not seem like there's a way to do this in a really modular and loosely coupled fashion which is otherwise something Fusion supports well. By modular and loosely coupled I mean that:
The example of registering GraphQL resolvers with Apollo has already been brought up. Another concrete example from our app is where we show a unified list of “events” originating from different downstream systems. When the user selects a single event we need to query one of those downstreams for the details, based on an identifier in the event ID. The nicest way to structure this is to have a handler for each type of downstream and just let the GraphQl query resolver run through each one until it finds one that can supply the details of that type of event. Yes, you can achieve this today in Fusion via the enhance, but it’s not very elegant and creates a harder coupling between things. I do not think the general pattern is really that niche, and I also don't think adding this feature is taking the Fusion API down any slippery slope as I find it hard to see what other kinds of “array like” tokens might be added: Fusion currently supports 1:1 bindings and we are simply proposing that it should also support 1:* bindings. For context, AFAIK many of the bigger DI frameworks for Java have this feature. As an example, the doc for "multibindings" in Guice (https://github.com/google/guice/wiki/Multibindings) provides a good illustration of why this pattern can be useful. If you think the specific solution proposed in this PR has issues, would you guys be open to discussing it in an RFC as Giancarlo mentioned? |
This adds a feature to create plugins where multiple values can be added to the same token and then be injected into the dependee as an array.
Usage example
Use case
We already use a similar approach to handle apollo resolver registration, though not as generalized as here.
Our use case might be a little specialised as our app contains multiple high level modules that can extend the core functionality, but we do not want to import directly from the modules in the core.
One such feature is custom permission checkers. Each module can add permission checkers for their custom types which will then be run every where they are needed.
Design decisions
There are several ways this could be implemented, I decided to not add an extra method to the fusion app but instead make the tokens special.
I did consider having a
addTo
or similarly named method for registration.Documentation
I did not any documentation for this yet, but will add if this is a feature that can be generally used.