Support composing components with differing render prop names

[erikthedeveloper]

Currently, we only support composing components which all have the same render prop name via props.renderPropName (defaults to children). This eliminates the possibility of composing together components with differing render prop names such as children, render, someCustomRenderPropName.

One potential, non-breaking, API I've come up with (open to suggestions for alternatives here 😄 ) is introducing a props.renderPropNames which would be an array of strings, being the same length as components, so that components[n]'s render prop would map to renderPropNames[n].

I came up with this as a non-breaking solution trying to remain consistent with renderPropName.

renderPropNames would take precedence over renderPropName if both somehow were present.

To better describe this, let me dump this test here that I was playing with locally. I was able to get this working fairly easily:

  describe('Configurable render prop names', () => {
    test('It supports composing components with differing render prop names', () => {
      const renderPropComponent = renderPropName => props =>
        props[renderPropName](props.value);

      const renderPropNames = ['children', 'render', 'customName'];
      const [AsChildren, AsRender, AsCustomName] = renderPropNames.map(
        renderPropComponent
      );

      const wrapper = mount(
        <Composer
          components={[
            <AsChildren value="one" />,
            <AsRender value="two" />,
            <AsCustomName value="three" />
          ]}
          renderPropNames={renderPropNames}
          children={results => <MyComponent results={results} />}
        />
      );

      expect(wrapper.find(MyComponent).prop('results')).toEqual([
        'one',
        'two',
        'three'
      ]);
    });
  });

Some other potential APIs that come to mind:

<Composer
  components={[
    {component: <AsCustomName value="three" />, renderPropName: 'customName'},
    <SimpleCaseStillWorks />,
    // ...
  ]}
  children={/* ... */}
/>

<Composer
  components={[
    <AsCustomName value="three" renderPropName="customName" />,
    // ...
  ]}
  children={/* ... */}
/>

[erikthedeveloper]

@jmeas I'm interested to hear your thoughts on supporting this or whether you've run into challenges already with this. I'll try to get my branch up as a PR soon, but wanted to get it out there for discussion before digging into it too far myself.

[jamesplease]

I'm alright with adding this if there's a real use case for it. Are you finding yourself trying to compose two or more render prop components with different render prop names, or is this a theoretical concern at the moment?

or whether you've run into challenges already with this

I only use Composer with the new React context API (and polyfill libs for the new API), as well as React Request, which all use children as the render prop name.

[erikthedeveloper]

if there's a real use case for it

That is the question to ask @jmeas! This was just something that came to mind to me more out of curiosity as I had a moment to sit down and play with react-composer 😆 . We do have a small handful of varying named render prop components in our code base, but I can't say that I've legitimately run into this pain point or have any compelling use case for this.

I think we can close this out and just note it as an interesting hypothetical. I don't feel like we need to bake in support for this right now though.

📝 Interestingly, I found that one simple way of accomplishing this was using Composer itself 🤔. The code below might be useful if someone runs across it and really wants to make it work. This passes as of 4.1.0 with no modifications to Composer.

test('It makes possible composing components with differing render prop names', () => {
  const renderPropComponent = renderPropName => props =>
    props[renderPropName](props.value);

  const [AsChildren, AsRender, AsCustomName] = [
    'children',
    'render',
    'customName'
  ].map(renderPropComponent);

  const wrapper = mount(
    <Composer
      components={[
        <AsChildren value="children one" />,
        <Composer
          components={[
            <AsRender value="render one" />,
            <AsRender value="render two" />
          ]}
          renderPropName="render"
        />,
        <Composer
          components={[<AsCustomName value="customName one" />]}
          renderPropName="customName"
        />
      ]}
      children={results => <MyComponent results={results} />}
    />
  );

  expect(wrapper.find(MyComponent).prop('results')).toEqual([
    'children one',
    'render one',
    'render two',
    'customName one'
  ]);
});

[jamesplease]

I think we can close this out and just note it as an interesting hypothetical. I don't feel like we need to bake in support for this right now though.

Sounds like a good plan.

---

In hindsight, I regret adding mapResult, because it wasn't added out of necessity. Every render prop component I can think of only passes a single argument to the prop. But now I can't bring myself to remove it 🙁

[erikthedeveloper]

Every render prop component I can think of only passes a single argument to the prop. But now I can't bring myself to remove it 🙁

I was pondering about that myself the other day. I have to imagine there are at least a handful of multi-argument render prop components out there especially in custom code bases... I wondered if it could be desirable to somehow preserve the signature of the render prop component (would mean deprecating mapResult). So that single argument (90% case) comes out as single value and mutli-argument render prop components come out as an array.

For example, I threw this together playing with the idea:

Enabling this (all other tests passing):

test('It sort of preserves render prop signatures for multi-argument render prop functions', () => {
  const wrapper = mount(
    <Composer
      components={[<Echo value="one" />, <DoubleEcho value="two" />]}
      children={results => <MyComponent results={results} />}
    />
  );

  expect(wrapper.find(MyComponent).prop('results')).toEqual([
    { value: 'one' },
    ['two', 'TWO']
  ]);
});

By only making this edit:

diff --git a/src/index.js b/src/index.js
index c62838b..0e95cbb 100644
--- a/src/index.js
+++ b/src/index.js
@@ -11,6 +11,10 @@ export default function Composer({
     return null;
   }

+  if (mapResult) {
+    console.warn('props.mapResult is deprecated.');
+  }
+
   /**
    * Recursively build up elements from props.components and accumulate `results` along the way.
    * @param {Array.<ReactElement|Function>} components
@@ -36,9 +40,9 @@ export default function Composer({
             // Remove the current component and continue.
             components.slice(1),
             // results.concat([mapped]) ensures [...results, mapped] instead of [...results, ...mapped]
-            results.concat(
-              mapResult ? [mapResult.apply(null, arguments)] : arguments[0]
-            )
+            results.concat([
+              arguments.length === 1 ? arguments[0] : Array.from(arguments)
+            ])
           );
         }
       }

Anyway, if we want to explore the idea I guess we aught to promote this to an issue. I just wanted to throw that out here for now 😄