Relying on function.name is brittle

I think relying on fn.name makes the API very brittle. The function name can (and usually does!) change during minification.

I understand .dispayName or explicitly specifying it as a "fake name prop" works as a more reliable fallback but I don’t think most people will be aware unless you force this. The name prop is also problematic because components might accept a legitimate name prop, and the API clobbers it.

Apologies if I misunderstood the API!

(Note: I know it says in the README to set displayName. But most people won't because it works without that in DEV.)

Here's a pure React version of this pattern that doesn't suffer from those drawbacks and in my subjective opinion it's clearer what's going on:

const MediaObject = ({
  image,
  heading,
  text
}) => (
  <Flex p={2} align='center'>
    <Box width={128}>
      {image}
    </Box>
    <Box>
      {heading}
      {text}
    </Box>
  </Flex>
))

const App = props => (
  <div>
    <MediaObject
      image={<Image src='kitten.png' />}
      heading={
        <Heading>
          Hello
        </Heading>      
      }
      text={
        <Text>
          This component keeps its tree structure but still allows for regular composition.
        </Text>      
      }
    />
  </div>
)

The input are explicit, there's no reliance on names, it can be strongly typed, and there are no questions about how nesting works and whether you can reorder them: it's just props.

(And it's not a library, just React :-)

Another possible way to express this in vanilla React is to make children prop be an object:

const MediaObject = ({{
  image,
  heading,
  text
}: children}) => (
  <Flex p={2} align='center'>
    <Box width={128}>
      {image}
    </Box>
    <Box>
      {heading}
      {text}
    </Box>
  </Flex>
))

const App = props => (
  <div>
    <MediaObject>{{
      image: <Image src='kitten.png' />,
      heading: (
        <Heading>
          Hello
        </Heading>      
      ),
      text: (
        <Text>
          This component keeps its tree structure but still allows for regular composition.
        </Text>      
      )
    }}</MediaObject>
  </div>
)

Thanks for the thoughts on this! I'd say right now the API is what I'm after, and the implementation needs some work.

Good point about fn.name – I'll remove that because it'll likely cause confusion.

The point of this API is to avoid custom prop definitions altogether. I tend to see a lot of components try to abstract out the layout and styling concerns with components that look like this:

<SomeHeader
  heading='Hello'
  description='Hi'
  buttonLabel='Go'
  onButtonClick={handleClick}
/>

In this example the custom props here are not tied to any data structures and require documentation and/or looking them up in order to use the component, which I don't think is great.

Instead, this library is aiming to provide an API like the following:

<SomeHeader>
  <Heading>Hello</Heading>
  <Text>Hi</Text>
  <Button onClick={handleClick}>Go</Button>
</SomeHeader>

With a team working with a common set of UI components, I think this API is a lot easier to use that defining "hole" props or object children – though your suggestion is arguably a much more React-like and explicit API.

Hope that makes sense, and I really would appreciate any thoughts on how to achieve this sort of API in a better way 😎

I'm struggling to understand why (2) needs less documentation than (1). I understands it looks clearer from the first glance but I need to do a lot of mental wiring to understand what actually happens.

Here's a list of ways (2) counters my expectations:

• Normally I can rename a component and all references to it, and things will work. However if I have a Heading in the same file as my usage of MediaObject, and then rename Heading to MyHeading, it breaks.

• Normally I can rename the public API of a component, and updating callers alone is enough. However if I rename the Heading argument to MediaObject to MyHeading, I can't pass the Heading component to it anymore.

These two issues are already very confusing to me. Imagine this function:

function power(x, y) {
  return x * y;
}

You can call it with any variables:

let a = 3;
let b = 5;
console.log(power(a, b));

The API in (2) is conceptually similar to enforcing that if x and y are ever renamed in power definition, local a and b in their callers must also be renamed, and vice versa. I find this to be a confusing limitation, especially in the context of JSX that normally works similar conceptually to function calls.

Even if x and y are named arguments you can still alias them:

let a = 3;
let b = 5;
console.log(
  power({x: a, y: b})
);

But here you can't.

Let's consider a specific example. Say both Heading and MediaObject come from npm. They work well together.

Then MediaObject 2.0 comes out, and it changes the "slot" to be called Header. If it was a prop you'd just pass a different prop:

- <MediaObject heading={<Heading />} />
+ <MediaObject header={<Heading />} />

Note this happens at the call site where you integrate both components. You don't need to change their source.

However, with the approach in this library, this doesn't actually work. We have to rename the argument now at its definition site:

- Heading.displayName = 'Heading';
+ Heading.displayName = 'Header';

What if you don't have easy access to the Heading code? What if it's unmaintained, or tricky to update? Well, you could do this:

function Header(props) {
  return <Heading {...props} />
}
Header.displayName = 'Header';

This component only exists so that MediaObject keeps working. Okay. Then somebody sees this code and thinks: unnecessary component indirection! Let's remove it so that the code can run faster! They just change it to re-export Heading directly, and the code breaks again.

(In fact that's exactly what a future React compiler might do. We're working on folding React functional components at build time for better performance. So if code relies on displayName for correctness, it might actually not work with the future React optimizations.)

You get the same kind of problem if two "MediaObjects" maintained by different teams agree on slot naming at first but then one of them renames the slot and the other doesn't. Now you have to wrap the component at least in one of the places.

I think you bring up some great points, and this library might not be for everyone.

I'll try to elaborate on my motivations a bit more and why I suspect people want an API like this...

Normally I can rename a component and all references to it, and things will work. However if I have a Heading in the same file as my usage of MediaObject, and then rename Heading to MyHeading, it breaks.

Renaming or using a different prop name would also break the functionality of a props-based MediaObject, right?

Normally I can rename the public API of a component, and updating callers alone is enough. However if I rename the Heading argument to MediaObject to MyHeading, I can't pass the Heading component to it anymore.

This is certainly not how this library is intended to be used, but, again, if you changed the prop name in one of your examples, it would break functionality.

In your example, using an img prop instead of image would mean that the content doesn't render.

// doesn't render because the prop name is incorrect
<MediaObject
  img={<Img src='kitten.png' />}
/>

With the macro-components example, passing an Img component instead of an Image component, similarly wouldn't render.

// doesn't render because the component is incorrect
<MediaObject>
  <Img src='kitten.png' />
</MediaObject>

For the first example, you can add prop type checking, but as a consumer, I still need to know the props that this specific component accepts. I can try to create a set of standards for what prop names should be used across multiple components, but that is limiting and hard to enforce.

macro-components is intended for use with a finite set of common UI components – i.e. a design system/component library where these macro-components would be a part of and ideally versioned together. That is, macro-components are meant to be tightly coupled with a specific set of compatible UI components.

You'll still need to know what components a particular component will accept, whereas with the props approach you need to know what props it accepts.

Managing and/or memorizing a common set of UI components seems simpler than enforcing a set of standards on how to name props.

Hope that makes sense, but let me know if that's still unclear

In your example, using an img prop instead of image would mean that the content doesn't render.

Sure. I'm not saying you won't need to change anything with props-based approach when props are renamed. I'm just saying that changing the callsite alone is enough with a props-based approach.

In a prop-based system with three components (App renders Image inside of a MediaObject) any renamings in MediaObject API can be fixed by changing code in App alone.

However, with approach taken by this library, changing MediaObject API forces you to change not just the App, but the Image too.

Does this distinction make sense? I'm not sure if you disagree that this is a concern or don't see the difference from a similar situation with regular props (which I believe is different).

I think I see your concern, but for where I'd use something like macro-components, I'd consider that a fair trade-off.

For example, if MediaObject made a breaking major change to use FancyNewImage instead of Image, I'd expect consumers of these components to make updates to wherever MediaObject and Image where used together, because they are coupled. That said, with a somewhat stable library of UI components, I don't see the need to ever make FancyNewImage.

Perhaps the MediaObject is a bad example because it is very general purpose, and I wouldn't suggest using macro-components for things that are intended to be used as generic, standalone components.

I think I might be misunderstanding the intended use from the readme.

For example, if MediaObject made a breaking major change to use FancyNewImage instead of Image, I'd expect consumers of these components to make updates to wherever MediaObject and Image where used together, because they are coupled

Why? My understanding is MediaObject doesn't know anything about Image (except the name that it choose for the "slot"). Does MediaObject actually depend on Image API anyhow? If not, then why are they coupled? If it is, can you show an example?

I guess I'm confused if MediaObject is ever supposed to work with two different Image implementations.

From your comments it seems like no (they're tightly coupled).

But from the API it seems like yes (because macro gives you Image as an argument so it looks intentionally "injectable" rather than hard-coded to one implementation).

If MediaObject is intentionally coupled to Image, I'd expect it just to import Image instead of getting it as an argument from macro inner function.

I guess I'm confused if MediaObject is ever supposed to work with two different Image implementations.

Generally, I'd say it's not intended to work with any other Image component, but it can, which might be a flaw in the implementation.

I think perhaps a better example would be something like the Bootstrap modal, which in HTML looks like the following:

<div class="modal" tabindex="-1" role="dialog">
  <div class="modal-dialog" role="document">
    <div class="modal-content">
      <div class="modal-header">
        <h5 class="modal-title">Modal title</h5>
        <button type="button" class="close" data-dismiss="modal" aria-label="Close">
          <span aria-hidden="true">&times;</span>
        </button>
      </div>
      <div class="modal-body">
        <p>Modal body text goes here.</p>
      </div>
      <div class="modal-footer">
        <button type="button" class="btn btn-primary">Save changes</button>
        <button type="button" class="btn btn-secondary" data-dismiss="modal">Close</button>
      </div>
    </div>
  </div>
</div>

When taking something like this and building it out as a React component, I often see this done:

<Modal
  title='Modal title'
  body='Modal body text goes here.'
  buttonLabel='Save changes'
  onClose={handleClose}
  onButtonClick={handleButtonClick}
/>

I don't think this is great, and I'd guess you'd generally agree here, but maybe not?

I suspect the reasoning for doing this is to abstract out all the nested DOM structure and styles required to make this modal look the way it's intended to, but it also means that it's using a heading, some text styles, and buttons from Bootstrap to achieve a certain look. The problem I see with this approach is that I have to learn arbitrary props like onButtonClick to know how to use this Modal component.

I suspect that this sort of API is a little easier to pick up – at least I find it nicer to work with:

import { Modal, Heading, Button, } from './ui-library'

const modal = (
  <Modal>
    <Heading>Modal title</Heading>
    <Button onClick={handleButtonClick}>Save changes</Button>
  </Modal>
)

In the latter, I already know about my Button component, because I use it in other places, and I know how to use its onClick prop. I only need to learn what components can and can't work with my modal.

One other part of your first comment that I don't fully understand is:

The name prop is also problematic because components might accept a legitimate name prop, and the API clobbers it.

Currently the API passed the name prop on, which could be not great for HTML elements that aren't form elements – is that what you're referring to?

BTW, thanks for the discussion – I struggle to figure out how to articulate the motivation and thought behind this idea, and this has been super helpful

I don't think this is great, and I'd guess you'd generally agree here, but maybe not?

I don't think I agree here. What is bad about it? Is it just that when people see attributes they think they can only pass strings?

The problem I see with this approach is that I have to learn arbitrary props like onButtonClick to know how to use this Modal component.

No, that's just an artifact of the props you decided to pick. You don't have to design the prop API this way.

The equivalent prop API to your last example would be:

import { Modal, Heading, Button, } from './ui-library'

const modal = (
  <Modal
    heading={<Heading>Modal title</Heading>}
    footer={
      <Button onClick={handleButtonClick}>Save changes</Button>
    }
  />
);

Yes, it is a bit more verbose. The nice thing about it though is that you can express more things with it. For example, if your modal footer is two buttons now, you know exactly what to do:

import { Modal, Heading, Button, } from './ui-library'

const modal = (
  <Modal
    heading={<Heading>Modal title</Heading>}
    footer={
      <React.Fragment>
        <Button onClick={handleButtonClick}>Save changes</Button>
        <Button onClick={handleCancelClick}>Cancel</Button>
      </React.Fragment>
    }
  />
);

Or if the footer becomes too large you can extract it into another component, the React way:

import { Modal, Heading, Button, } from './ui-library'

const modal = (
  <Modal
    heading={<Heading>Modal title</Heading>}
    footer={<MyModalFooter onSave={handleSave} onCancel={handleCancel} />}
  />
);

Note that I went from passing a Button to passing a Fragment with two buttons to passing a MyModalFooter (which might render those and even contain state).

The Modal component doesn't care. It provides two "slots" via props (heading and footer) and doesn't really care what you put there.

It seems like the API you're suggesting doesn't work with composition this way (intentionally?)

In the latter, I already know about my Button component, because I use it in other places, and I know how to use its onClick prop. I only need to learn what components can and can't work with my modal.

Do you agree that the API I propose for such scenarios in this comment satisfies this constraint?

Currently the API passed the name prop on, which could be not great for HTML elements that aren't form elements – is that what you're referring to?

Maybe Button component created by a designer has a name prop (e.g. <Button name="cancel" />). It doesn't necessarily have to do with HTML fields. But yes, the problem is that name is a common userland prop so overloading its meaning in a library leads to surprising outcomes.

No, that's just an artifact of the props you decided to pick. You don't have to design the prop API this way.

The issue I see isn't so much that someone with experience with React would do this, it seems like something I see more often with beginners/people new to React FWIW.

Thinking on this a little more, in the context of how I'd imagine people would use this, one thing that your examples expose that I wouldn't want to do is allow for any node type to be passed to the heading prop. For example, if I've designed a Modal component, I would not want someone to pass an Image component to the heading prop.

// e.g. I don't want this to happen because it's not how it was designed to work
<Modal
  heading={<Image />}
/>

This approach is way more flexible that what macro-components allows for, but that's part of the point.

It seems like the API you're suggesting doesn't work with composition this way (intentionally?)

Probably, yes?

What this API is sort of trying to do is turn the normally opaque data structure of props.children into a stricter API, but one that is easy to read and use. Maybe this is still a very bad idea, but it seemed like something worth exploring

For example, if I've designed a Modal component, I would not want someone to pass an Image component to the heading prop.

You don't actually enforce this though, do you? If I wanted to pass an Image I could pass it in a component called Button. I'd expect that in a large enough codebase that's exactly how people will "work around" the API:

function ButtonPoser(props) {
  return <Image {...props} />
}
ButtonPoser.displayName = 'Button';

This will fool the library into thinking it's rendering a button.

On the other hand, I'd expect people to get confused why they can't render an "indirection" without hacks. Even though you want people to only render buttons, introducing an intermediate component is still helpful in many cases. For example:

function LikeButton(props) {
  return <Button onClick={Actions.handleLike}>{LikeStore.getCount()}</Button>;
}

Should it really be disallowed to pass a LikeButton in place of a Button? Technically it does render to a Button. For anyone who's used to React, I think it would be surprising that you're not allowed to introduce an abstraction layer without hacks like I described above (reassigning displayName to "lie" about what you're passing).

Probably, yes?

In this case it would make sense to me if the "macro" component had to explicitly reference the inner components instead of them being "injectable". Then you express the dependency in a direct way.

Usage could look like this:

const modal = (
  <Modal>
    {M => 
      <>
        <M.Heading>Modal title</M.Heading>
        <M.Button onClick={handleButtonClick}>Save changes</M.Button>
       </>
    }
  />
);

That would make it explicit that modal components are preconfigured and can't possibly be changed without changing the Modal itself. (Which goes against React composition model but indeed gives you more strictness.)

Anyway, I just wanted to raise those concerns in case they weren't obvious. :-)

It's cool if we disagree. I think my main concern is just that this is a very opinionated way of doing things that breaks some React composition model expectations, and I'd feel better if this was explicitly called out somewhere.

I'd like beginners to understand "the React way" of doing this before they learn this library so that if they adopt it, it would be a conscious decision with an understanding of its tradeoffs.

Hmm. Considering that everything is preconfigured you could actually allow this API:

const modal = (
  <Modal>
    <Modal.Heading>Modal title</Modal.Heading>
    <Modal.Button onClick={handleButtonClick}>Save changes</Modal.Button>
  </Modal>
);

Or, for extra terseness,

const { Heading, Button } = Modal;
const modal = (
  <Modal>
    <Heading>Modal title</Heading>
    <Button onClick={handleButtonClick}>Save changes</Button>
  </Modal>
);

This makes it obvious that Modal is in control over its "sanctioned child components", doesn't let consumers hijack them, and looks pretty clear.

It also doesn't rely on name or displayName at all, and thus doesn't need the name prop.

<Banner>
  <Banner.Heading>Hello</Banner.Heading>
  <Banner.Subhead>Subhead</Banner.Subhead>
</Banner>

Since Banner.Subhead would really be set to Heading component, you'd get propTypes validation and the right names in DevTools and warnings.

What do you think?

The concerns over beginners choosing this without understanding a more normal React-based approach makes a ton of sense, and I'll definitely add your PR + some language around that to the README.

Funnily enough, the original API I had going is much more inline with what you've suggested in the last comment (https://github.com/jxnblk/macro-components/blob/d30866b868dfd7fb60e8a7a01a91569c6f1513f3/README.md), but the implementation had some (probably obvious) performance issues.

I'm curious about what the authoring experience would look like for a component that works like this:

<Banner>
  <Banner.Heading>Hello</Banner.Heading>
  <Banner.Subhead>Subhead</Banner.Subhead>
</Banner>

(I get how to manually make a component that does this, but what's the simpler way?)

I suspect we probably don't disagree on everything here, and I'd like to figure out a good/simple way to support building out components like what macro-components is trying to do, and this conversation has got me thinking about the problem in new ways – so thanks again :)

I filed https://github.com/jxnblk/macro-components/issues/8 with my API proposal.

Love the discussion and the new API proposal!

I'm still a bit torn on the general idea of macro-components. Writing components like this looks so nice! But at the same time the fact that children of a macro component aren't all treated equally somehow worries me, even though I should probably just accept it as the component API the same way I accept normal custom props. With the new API I could definitely see myself using this. 👍

I was suggesting the Banner.Heading pattern in another project/issue https://github.com/zeit/styled-jsx/issues/394#issuecomment-362909647 @gaearon is the implementation from the gist what you would go for?

v2 is published with a new API based on #8 – but leaving this open for any additional discussion