Add connected components

[etalisoft]

Connected Components

Why?

Currently all components are dumb components. Given a component hierarchy A->B->C->D->E. If Component E needed access to state.name then components A, B, C, and D would each need to be updated to pass along state.name. This causes components to become tightly coupled, or written generically so that all state and actions are handed down to every child component, because of the "you never know when you'll need it" mentality.

The Solution

Natively support both dumb components and connected components.

Normal dumb component signature:

const Component = (props, children) => (
  <div>Hello</div>
);

Connected component signature:

const ConnectedComponent = (props, children) => (state, actions) => (
  <div>Hello, {state.name}</div>
);

This solution does not require use of either a Higher Order App or a Higher Order Component.

How does it work?

In hyperapp's render function, whenever the view is processed, the resulting object tree is traversed. Any child node that is a function is called with the globalState and wiredActions.

This change raised the gzip size by ~30 bytes.

[codecov]

Codecov Report

Merging #604 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #604   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines         139    143    +4     
  Branches       42     43    +1     
=====================================
+ Hits          139    143    +4

Impacted Files	Coverage Δ
src/index.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0b765d...41c1cbd. Read the comment docs.

[SkaterDad]

Very interesting idea! Will take a closer look at this later.

While the framework size increases, I wonder if this would lower the actual application bundles in some cases.

EDIT:

My big concern with this approach is the performance hit. We're already traversing the entire view before this new inflate function is called, so you're effectively doubling the work on each render. If this idea has traction, maybe there's a way to accomplish this within the 1st iteration?

[Swizz]

My question at this stage is more pragmatic. What is the benefits of Connected Components regarding of the explicit use of state and actions properties ?

const Component = (props, children) => (
  <div>Hello, {props.state.name}</div>
)

const view = (state, actions) => (
  <div>
    <Component state={state} actions={actions} />
  </div>
)

Yes, you will need to add state={state} actions={actions} to each tree step.
Why is this a problem for you ? Only about DRY ?

[jorgebucaran]

@jason-whitted I would prefer if people created userland solutions for this like you did with hyperapp-connect.

[etalisoft]

Hyperapp applications consist of three interconnected parts: the State, View, and Actions.

From the moment of instantiation an app binds these pieces together. The view is composed of a hierarchy of components. Every instance of each component in the app is directly tied to the state and actions; however, currently they do not have native access to them.

Due to this limitation it forces the parent component to be concerned with the business of the child component. This causes tight coupling and a violation of separation of concerns. To bypass this, component developers will be forced to relay action and state down through every component they create to every child.

@SkaterDad

My big concern with this approach is the performance hit. We're already traversing the entire view before this new inflate function is called, so you're effectively doubling the work on each render. If this idea has traction, maybe there's a way to accomplish this within the 1st iteration?

I would love to do handle this in the 1st iteration, but my unfamiliarity with the codebase is slowing down progress. Could you point me in the right direction? It appears to me that the view does one of two things:

1. Simply returns an object map (without the use of h), in which case the inflate call is the first iteration.
2. Invokes a nested chain of h functions, which returns an object map. Since the h functions are not bound to the app instance it cannot carry the state/actions down to the object it creates.

I would assume the patch function then could be considered the 1st iteration. Would this be a better place to put the inflation logic?

@jorgebucaran

I would prefer if people created userland solutions for this like you did with hyperapp-connect.

My "userland solution" is a hack at best. You even had questions about why the key was needed. Requiring a HOA and an HOC and the cognitive load required to do a common task goes against one of the main points of Hyperapp.

We have aggressively minimized the concepts you need to understand
Out of the box...all with no dependencies

[etalisoft]

I have updated the code to no longer prematurely recursively inflate before the patch process. It now happens during the patch process.

[okwolf]

Why the name inflate exactly? 🤔

[etalisoft]

A node has children. Some of the children may be functions instead of other nodes. The inflate function is converting them. Hydrate was already used. Inflate sounded good. Haha, have a better name?

[okwolf]

@etalisoft A node has children. Some of the children may be functions instead of other nodes. The inflate function is converting them. Hydrate was already used. Inflate sounded good. Haha, have a better name?

Not sure, maybe renderChildren? 🤷‍♂️

[etalisoft]

Or connect? I don't know why I didn't just use that begin with.

[jorgebucaran]

I have nothing against the name. It reminded me of this from one of my favorite childhood games:

@etalisoft Can you try implementing this touching only h?

[etalisoft]

I renamed inflate to connect. I also found that by converting the for (var i = 0; i < array.length; i++) to for (var i in array) I could cut out 20 bytes from the gzip. Which means this patch only has a net gain of ~10 bytes. ;)

[etalisoft]

@jorgebucaran

Can you try implementing this touching only h?

I thought long and hard on trying to do that. It was the first logical place I tried. I touched on the difficulties previously:

Since the h functions are not bound to the app instance it cannot carry the state/actions down to the object it creates.

I'm not sure how I could accomplish this, since the h function isn't derived from the app instance.

[jorgebucaran]

Why are you touching this? @etalisoft

[etalisoft]

@jorgebucaran
Given the focus on the end product file size, I was looking for ways to make up for some of the bytes that I added. Obviously this can get reverted as it has no impact on the Connected Component code.

var arr1 = ['a'];
arr1[100] = 'b';
for (var i = 0; i < arr1.length; i++) 
	console.log('arr1', i);
// outputs 100 times
  
var arr2 = ['a'];
arr2[100] = 'b';
for (var i in arr2)
	console.log('arr2', i);
// outputs twice

Based on the above logic, in addition to savings file size, it seemed it could have the added benefit of possibly reducing iterations.

[jorgebucaran]

Let's implement it first, then worry about file size. Please revert it.

[jorgebucaran]

arr2[100] = 'b';

Haha yeah, but that can't happen and regular for loops are faster than slow foreach loops.

[jorgebucaran]

@etalisoft It's a tough one. What else can we do to mitigate the performance hit?

[etalisoft]

@jorgebucaran Is there some kind of performance load test that the team has been using? Maybe something that renders thousands of children and fires state changes. If not, maybe it's the next thing I build :)

I'd like to compare the before and after performance. I think the performance impact will be extremely minimal.

[jorgebucaran]

@etalisoft You are iterating over children twice every time.

[etalisoft]

Not any more! 😉

[jorgebucaran]

@etalisoft You forgot to revert the for..in loops to for loops.

[zaceno]

Since context was brought up again (not my fault this time! 😅 ): If Redux's connect function is based on React's context, wouldn't it make sense to have context in Hyperapp, and solve the connected-components goal through that?

PS
The only reason I didn't PR my HOA yet is because I'm like 99.9% sure it will be rejected 😄:

@jorgebucaran I would prefer if people created userland solutions for this like you did with hyperapp-connect.

[jason-whitted]

@zaceno @jorgebucaran
I was just playing around with the entire context idea, just to be able to compare and contrast.

I created a modified version of app that provides context as a third parameter to components.
Components then have the signature:

const Component = (props, children, context) => <div>{children}</div>;

The context is just a plain object. It is cloned from the parent object. If a component wants to add something to the context for it's children they just do it in the function body.

const Parent = (props, children, context) => {
    context.apiKey = "abc123";
    return <div>{children}</div>;
}
const NestedChild = (props, children, context) => <div>{context.apiKey}</div>;

With this ability I then created a Provider and connect that works almost identical to react-redux.

If you want to take a look, I put it on CodePen.

This is obviously a completely different solution.

// `current` solution
const Component = (props, children, { getState, getActions }) => ...

// `context` solution
const Component = (props, children, context) => ...

The current solution should have 0 performance impact. Since the current object is generated and frozen once during the initial app mounting and then just past around.

The context solution is extremely flexible, and it pretty much works the same way as React. Unfortunately, this does not come for free since the context has to be cloned from the parent.

[jorgebucaran]

If in order for this to exist in userland without severely patching the app function we need to introduce a feature like React's context, then let's look into it. We can't simply have all the features, but we can add something new that opens the door to new, exciting and generalized possibilities.

[zaceno]

@etalisoft

The context solution is extremely flexible, and it pretty much works the same way as React. Unfortunately, this does not come for free since the context has to be cloned from the parent.

Nice solution for context! And you're right -- there is a penalty to it.

Not sure if you're deep-cloning or shallow, but I think shallow should be enough and the impact should be negligible. Still: it's there. ..and would annoy anyone who doesn't want/use it.

Your connected-components approach is essentially free, and as such "harmless" to those who don't care about it. On the other hand: not as flexible as context.

[frenzzy]

I agree that hyperapp must have a way to pass things (global state/actions/context/etc) down to components without the need to do it manually (using props through the whole tree). I also agree with @jorgebucaran that core should not implement all the features but just allow you to extend it in userland.

In my mind we have two ways how to achieve this (an ability to communicate parent and child components without modifying components tree in the middle):

1. Context object

For example via component signature:

const Component = (attributes, children, context) => <div />
// or
const Component = (attributes, children) => context => <div />

But I do not see a way how to make it explicit and simple to understand/use for users. Example above with context.apiKey = "abc123" looks very ugly/untestable/etc. React.js changed context api because its uncler.
By the way

2. Global actions

For example with one of the following component signatures:

const Component = (attributes, children, actions) => <div />
const Component = (attributes, children) => (actions) => <div />
const Component = (attributes, children, state, actions) => <div />
const Component = (attributes, children) => (state, actions) => <div />

By using global actions you can solve the same problems which context solves. But actions already described by hyperapp and you know how to use them. And you also can implement your own mutable context based on actions or anything else.

By the way, React.js team changed context api trying to make it more explicit but they do not have global actions.

So, IMO we should approve only something like #604 to be in the core.

[zaceno]

@frenzzy And you also can implement your own mutable context based on actions or anything else.

This is not exactly true. Context isn't necessarily about the state, but about where in the component tree a component is being rendered -- not something you can keep in state, or use actions to update. Because the second you use an action... boop! The tree is being rendered again.

[Mytrill]

@frenzzy Have you checked out hyperapp-context? Would you agree that such an API would be an elegant solution for 1. that has much more possibilities than 2.?

[frenzzy]

@zaceno under the hood context is just an object which is available in all components. You can add any features to it to behave as you want. For example on a component level you can detect where you are and add this into to context.

Context implementation example using global action:

const context = { key: 'value' }

const state = {}
const actions = { getContext: () => context }

const ChildComponent = () => (state, actions) => {
  const context = actions.getContext()
  return <div>context: {context.key}</div>
}
const ParentComponent = () => <div><ChildComponent /></div>
const view = (state, actions) => <main><ParentComponent /></main>

app(state, actions, view, document.body)

or you may use higher order component:

const withContext = (component) => (attributes, children) => (state, actions) =>
  component(attributes, children, actions.getContext())

const YourComponent = withContext((attributes, children, context) => <div />)

[Mytrill]

@frenzzy As far as I understand, in hypperapp-context's case, any value set on the context is only available to the children of this component, not to the entire tree, as such I don't think you can (easily) implement it with an action/state slice.

[frenzzy]

@Mytrill it is solvable with a knowledge about component executing order. For example how implementation of new React.js context API may look in Hyperapp:

function createContext(defaultValue) {
  let value = defaultValue
  return {
    Provider(attributes, children) {
      value = attributes.value
      return children
    },
    Consumer(attributes, children) {
      return attributes.render(value)
    }
  }
}

const ThemeContext = createContext('light')

const view = (state, actions) =>
  <main>
    <ThemeContext.Provider value="dark">
      <div>
        <ThemeContext.Consumer render={val => <div>{val}</div>} />
      </div>
    </ThemeContext.Provider>
  </main>

app({}, {}, view, document.body)

Demo: https://codepen.io/frenzzy/pen/oEyGeJ?editors=0010

It does not work right now just because component execution order must be top to bottom, but currently components are executed in the opposite order. Lazy components PR #606 will solve this problem.

[zaceno]

@frenzzy Yes, that implementation looks more workable. It works because it does not use state/actions to read & create context. That's the only point I was making (and I think @Mytrill ).

Like you say, it requires "lazy components". Actually, your implementation of context, when you include the lazy-components aspect, is technically pretty much how my hyperapp-context hoa works -- except the API is totally different of course 😄

[jason-whitted]

@jorgebucaran @frenzzy @Mytrill @zaceno

I took @frenzzy's createContext idea:

// Could be the output of a hyperapp-context package
const createContext = defaultValue => {
  let value = defaultValue;
  return {
    Provider(attributes, children) {
      value = attributes;
      return <div>{children}</div>;
    },
    Consumer(attributes, children) {
      return attributes.render(value)
    },
  };
};

Created Provider and connect using the context:

// Could be the output of a hyperapp-connect package
const { Provider, Consumer } = createContext({});
const connect = (mapStateToProps, mapActionsToProps) => BaseComponent =>
  (props, children) => {
    const render = ({ state, actions }) =>
      <BaseComponent {...props} {...mapStateToProps(state)} {...mapActionsToProps(actions)} />;
    return <Consumer render={render} />;
  };

The end result is context AND connected components -- and the only thing that needs to be added to the hyperapp core library is #606 (@frenzzy's Lazy Components) and #611 (@titouancreach's fix minify).

import { h, app } from 'hyperapp';
import { Provider, connect } from 'hyerpapp-connect'; // uses hyperapp-context internally
const state = { value: 0 };
const actions = {
  value: value => state => ({ value }),
};

const Component = (props, children) => <div />;
const mapStateToProps = state => ({ value: state.value });
const mapActionsToProps = actions => ({ setValue: actions.value });
const ConnectedComponent = connect(mapStateToProps, mapActionsToProps)(Component);
// The ConnectedComponent would receive the following props:
//   name: string
//   value: number
//   setValue: function

const view = (state, actions) => (
  <Provider state={state} actions={actions}>
    <ConnectedComponent name="bob" />
  </Provider>
);

Here's a working CodePen.

[jorgebucaran]

@etalisoft The end result is context AND connected components -- and the only thing that needs to be added to the hyperapp core library is #606 (@frenzzy's Lazy Components) and #611 (@titouancreach's fix minify).

Meaning this PR is no longer relevant? Or did I misunderstand what you said?

And why #611? What difference did it make?

[etalisoft]

@jorgebucaran this PR is no longer relevant?

Correct, if lazy comments components gets added then this PR is not necessary.

why #611?

The minified code was not properly relaying props down to child nodes. The unminified code did not have this problem. When I incorporated #611 the minified code worked properly.

[jorgebucaran]

Correct, if lazy comments gets added then this PR is not necessary.

I know someone who loves lazy comments.

[okwolf]

@etalisoft Correct, if lazy comments gets added then this PR is not necessary.

comments === components?

[etalisoft]

@jorgebucaran lazy comments

Ha 😃 That’s what I get for typing on my phone.

Additionally, lazy components makes #615 irrelevant also

[okwolf]

I think we are fast approaching a major decision point for this project. Right now we have a pretty equal compromise between the simple/functional/verbose and complex/imperative/performance/pragmatic sides. If we choose to support connected components - that would be a shift toward the complex side in order to make certain things easier and more like React with somewhat stateful components.

My vote is to keep things simple in core, unless we can come up with another approach that is simple made easy 👌

[jorgebucaran]

@okwolf What about lazy components? It closes #604 and #615.

[okwolf]

@jorgebucaran What about lazy components? That closes #604 and #615.

That approach is more flexible and allows for the community to decide how to wire their components. Maybe some awesome approach (à la selectors) will emerge. Probably it's something no one has even thought of yet.

[etalisoft]

@okwolf Selectors, like reselect? If so, I’ve already codepenned that in one of these threads. They already work and pair perfectly with Provider/connect model listed earlier.

[okwolf]

@etalisoft Selectors, like reselect?

Conceptually, yes. I'd love to see an approach that improves on Reselect by integrating more tightly with the VDOM engine. This would give some of the same ergonomic improvements as the shorthand syntax for actions has over reducers.

[titouancreach]

@etalisoft The minified code was not properly relaying props down to child nodes. The unminified code did not have this problem. When I incorporated #611 the minified code worked properly.

Wow, just in time 👍

[jorgebucaran]

@etalisoft I think we can close here then?

[etalisoft]

@jorgebucaran Yes, #606 is a more flexible solution than this PR. This PR can be closed.

[jorgebucaran]

Closed in favor of #606.