Add DI HOC RFC

[shannonmoeller]

Rendered

Proposed implementation:
fusionjs/fusion-core#290
fusionjs/fusionjs#163

[IcculusC]

perhaps we could introduce an RFC in fusion-core that adds an optional argument to createPlugin's argument map that defines the behavior of exposing a plugin via this proposed HOC

something like expose where false disables exposing via the HOC, true naively exposes it, and a function defines any setup behavior and returns the service, that could both help to alleviate this point and add more utility and "safety" to DI

[shannonmoeller]

Perhaps it would be better to allow a mapServicesToProps callback argument similar to react-redux's connect function.

[lhorie]

Adding comments about technicalities here for posterity.

We want fusion-core to be agnostic of rendering library (therefore suggestions about adding react-specific arguments/options/APIs to fusion-core will likely require design refactoring/reconsideration)

If we want to specify the context property name via the withFusion API, that string needs to be propagated to something like this https://github.com/fusionjs/fusion-plugin-i18n-react/blob/master/src/plugin.js#L31 somehow. Not entirely sure how yet.

Alternatively, we could make the token the owner of the string name by allowing the token to have arbitrary or semi-arbitrary data attached to it. Then both Provider and HOC for a implementation would draw the context property name from there.

There are some known issues with regards to React, specifically:

• HOC prop pollution isn't really ideal (e.g. there's nothing stopping another HOC from clobbering https://github.com/fusionjs/fusion-plugin-csrf-protection-react/blob/master/src/hoc.js)
• It may make more sense to provide a render prop style API instead of an HOC

[shannonmoeller]

fusion-core to be agnostic of rendering library

Ideally this change could happen 99.9% in fusion-react.

HOC prop pollution isn't really ideal

Agreed. We could look at render props. But, for container components, I'm not sure how much this is an issue. Especially if the HOC lets you pick arbitrary values for your deps keys:

const Root = withServices({megaLogger5000: LoggerToken}, ({megaLogger5000}) => (
  <button onClick={() => megaLogger5000.info('clicked')}>Foo</button>
));

[shannonmoeller]

I've been playing with a prototype just to see what would need to happen to make this possible. Right now, I've identified a blocker in fusion-core in that the map of resolved plugin values isn't exposed.

https://github.com/fusionjs/fusion-core/blob/master/src/base-app.js#L114

[ganemone]

I've been playing with a prototype just to see what would need to happen to make this possible. Right now, I've identified a blocker in fusion-core in that the map of resolved plugin values isn't exposed.

This was intentional, exposing this runs the risk of a leaky abstraction in that anyone with a reference to app could tamper with the resolved dependencies. I think the risk here is pretty low, and we can only do so much to prevent people from shooting themselves in the foot so I wouldn't be averse to exposing it at least in an undocumented way. Ideally it is only accessed by app sub classes.

[shannonmoeller]

If possible, I'm thinking that the createInjectorPlugin stuff would happen in the fusion-react subclass of App and that the only thing that's truly public would be withPlugins.

[shannonmoeller]

HOC prop pollution isn't really ideal

This issue has been further solved by the addition of the optional mapServicesToProps callback.

[shannonmoeller]

@ganemone I think I've addressed the issue of the leaky abstraction. Would love your input:

https://github.com/fusionjs/fusion-core/pull/290/files#diff-0bcad607e1946bd4ccecb24f8b223b68R275