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

[Discussion] Plugin Composition #733

Open
dahlbyk opened this issue Sep 1, 2017 · 6 comments
Open

[Discussion] Plugin Composition #733

dahlbyk opened this issue Sep 1, 2017 · 6 comments

Comments

@dahlbyk
Copy link
Contributor

dahlbyk commented Sep 1, 2017

Griddle version

1.x and/or 2.x

Expected Behavior

Plugins and components compose well.

Actual Behavior


Thoughts on how this situation can be improved?

@dahlbyk
Copy link
Contributor Author

dahlbyk commented Sep 1, 2017

For starters, we could accept plugins as functions (reducers, basically). The result of the function, called with the initial/current config, would be treated the same as a static plugin.

This may be sufficient to allow enhancer/container components to be composed instead of replaced.

@dahlbyk
Copy link
Contributor Author

dahlbyk commented Sep 20, 2017

@sel129 ran into this in #741:

I have a situation where I have created 2 plugins. Both have a RowEnhancer. I expect that the RowEnhancer from both plugins will be applied to the Row Component.

It's reasonable to expect enhancers to build on each other, rather than be replaced, but changing that in 1.x would be a breaking change. Two ways occur to me to implement this without a breaking change:

  1. Implement the plugin function idea above, which would allow a plugin to wrap the existing enhancer:

    <Griddle
      plugins={[
        { components: { RowEnhancer: OC => props => <div>a <OC {...props} /></div> } },
        ctx => ({
          components: {
            RowEnhancer: OC => ctx.RowEnhancer(props => <div>b <OC {...props} /></div>)
          },
        }),
      ]} />
  2. Update the enhancer composition logic to accept enhancers as an array of components, which would be composed with each other instead of replaced.

    <Griddle
      plugins={[
        { components: { RowEnhancer: [OC => props => <div>a <OC {...props} /></div>] } },
        { components: { RowEnhancer: [OC => props => <div>b <OC {...props} /></div>] } },
      ]} />

Both of these would render <div>a <div>b <Row {...props} /></div></div>.

Between the two, I think I prefer the former, as it seems more deterministic.

@dahlbyk
Copy link
Contributor Author

dahlbyk commented Sep 24, 2017

FYI, I am hacking around this weekend on implementing support for plugins as functions.

This was referenced Oct 14, 2017
@shorja
Copy link

shorja commented Oct 23, 2017

@dahlbyk I've been thinking about the idea of plugins as reducers and I'm starting to like it. My initial concern is that it allows plugins to do ... like anything at all since they are getting the full Griddle configuration up to that point when they are run, though I suppose it doesn't really matter since its up to devs to decide what is appropriate.

On a basic level, each plugin would pretty much just add itself to the overall state/config in the same way that the Griddle initialization loops do, but if you needed finer grained control you would have it if necessary.

I'm working on changes to the selectorsUtils that should make a 'by plugin' adding of selectors easier.

@dahlbyk
Copy link
Contributor Author

dahlbyk commented Oct 23, 2017

That's exactly the idea. And if a plugin is provided as a plain object, we will continue to merge it into the configuration using current semantics (components get replaced, reducers get combined, etc).

Does the composeSelectors logic actually need to change, other moving the step to flatten the core/plugin selectors up into the plugin reducer? Ultimately applying plugins results in a final named list of selectors, from which your dependency Map gets created.

@shorja
Copy link

shorja commented Oct 23, 2017

Not explicitly, however I am adding some more features to the selectors and they way I'm doing it should make it very easy for reduceable plugins to add their own selectors onto the pile. It also may in the future enable fancier intermediate changes to the selectors by each plugin. I'll share it when its done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants