Code Crimes For Good Component API

I'm Siddharth, and I work on the design engineering team at GitHub. I'm going to talk to you about some code crimes we do to create good component APIs. Before we get there, let me ask you, what makes a good user interface? The kind of words that I hear a lot are, it should be intuitive, it should be accessible, and it should be consistent. If you know how to use one part of the application, you can guess how to use another part, because it's all consistent.

Now, I work on the React side of things. I work on this component library that we use to build other pages on GitHub, and this is a question that I think about, which is what makes a component API interface good? Because that's the thing that your users, the developers, are consuming. And it's kind of the same thing. It should be intuitive to read and author code, it should be accessible by default, and it should be consistent. So, by consistent, I mean it should have a small API surface area. If you know how to use one component, you can guess how to use another component, because the API is consistent. So, that's the goal, at least. But it's not always easy. Sometimes it's not in the good, happy parts of React, and that's where the crimes come in. So, let's look at a few.

I have this component. It's called AnchoredOverlay, and it's called AnchoredOverlay because it opens an overlay which is anchored to this button. So, AnchoredOverlay. And the way this button, the way this component works is that you have to manage its state. So, you pass open, you give it onOpen and onClose function, and then you give it a render. So, this is a render prop. You can render the element inside. In this case, I'm rendering a button. And then it passes you some props that you're expected to pass through to the element. So, there are things like, I have props in this. There's some styles for this AnchoredOverlay. So, kind of a simple API, not a lot going on. And then inside it, you can put children, which get rendered there. So, good component.

Now, the place where we might use this component is, this is a pull request page from GitHub. And you see there's a bunch of menus here.

You can assign labels, you can assign assignees to a pull request, and this looks like a good use case for AnchoredOverlay, because there is an overlay, and it's anchored to this button.

Now, if I have to build this component, the way I would go about this, I use the AnchoredOverlay. I have to manage its state, so I say open. I'm going to do a set state, and then just set open true and unopened and set open false and unclosed. And in rendered anchor, I can use the button component from the component library, pass through the anchor prompts. And we have this design convention that if a button opens a pop-up or a menu, it shouldn't just look like a normal button. It should have this indication. So we have a training icon and carrot is the default, so triangle down icon from the icon library. But in this case, it could also be this giver. And I'm going to tweak this button so that it looks like a bear. So I like it. It looks pretty good here, and it would work. The overlay is anchored to the button. Everything's good.

Now, the thing I don't like about this, is that the the API is not that nice. So anchored overlay is perfect, but if I have to create this menu, and I'm trying to create this into one component, and bake some of these decisions in, I, first of all, maybe I'll call it something like action menu because it's a menu of actions, and I don't really want the default to be status managed. The component should be smart enough to do this by default that when you click, it opens the menu. So I'm going to remove this. And the next thing that I would remove is this render prompt API. Again, I want this to be smart enough that it knows what props to pass, and then it can pass on its own. So this is kind of how it works. Trailing icon, again, it's a design convention that we already know, and we want people to follow, so it would be nice if it's baked in as the default. So I'm going to say action menu dot button. This is a button that you use with action menu and it has the triangle, the carrot, all be baked in. And finally, I'm going to create action menu dot overdate to adapt the contents because I'm going to add some props like width is medium. So this looks good to me. Let's jump into the implementation. The implementation of the action menu is kind of boring. It just renders its children. There's nothing special that it does.

Menu button is more interesting. So it's a component which renders the button, and you see that we're already baking in the trailing icon, so this is the default trailing icon. But then we also passed on props. So if you wanted to customize the trailing icon and put this gear icon, you can do that. And the way we create this API or action menu and action menu button is, it's kind of silly, but every component in React is a function, like it calls a function, and every function in JavaScript is also an object, which means you can get away with things like attaching one component to another. So you can say action menu dot button is actually menu button. So when you render this and you say action menu button, it actually calls this function. So it's awesome. And then you have a menu overlay where we use the anchored overlay, the component I showed first, and so it still uses anchored overlay under the hood, which manages all the state, but when you're creating this menu, you don't have to really worry about all of that.

So now when I'm looking at this component and trying to implement it, the state stuff is easy, that's fine, but then I have this render anchor, which is, I need a component to pass down these props too, right? That's how it works. I need to render this component, and the API that we've created is this is the button and this is the overlay. So we need some way of actually sending this action menu button into the overlay and React doesn't really have a way of communicating between siblings. Once one child cannot tell its sibling that here is what you need. So how do we do that? The React way of doing that would be to pull this up to the parent, right? React likes data to be passed from the parent down to the children. And then if you want the opposite way, you have to pass a callback to the child and the child can call that function. So that's what we do. We pull anchor up to the parent. So now action menu has a state, it defines anchor. And then we create a context, the pass both anchor and set anchor and wrapped children so that both menu and overlay can access this. Inside the menu we say, if anchor is not defined, well then that's define it. Set anchor and here's the button that we saw. And then the props that you pass get passed down to the button. So that works. And then we return null because this button doesn't really need to return anything. The anchor is actually rendered by the anchored overlay. So inside menu overlay we use context, we get the anchor from there and we do a render anchor. So a little strange but not too wild. This is the way you would abstract state up into a parent and then pass it down to all the siblings that need it. And this works well, like now if I reload you'd see that the assignees, the menu button doesn't render anything because menu overlay renders it. So it renders both the button and the overlay.

Now, the interesting thing is on client side it's all cool but if I do this with server side rendering, there's a funny thing that happens. You see, there's a bit of a lag here, right? And that's obviously not good and that's happening because let's say we do a server side render and then pause it. The first render on the server, the anchor state was null and then when the anchor is set, like when the menu button renders with set anchor and cause a re-render and that will set this render anchor for the overview. So we kind of need two renders though and if you're in server side render then the first time you render you would ship that HTML to the client and even if the second render happens after that, after set anchor, it doesn't matter because the HTML has already been shipped. And now on the client, the JavaScript bundle would arrive, react would hydrate and then it would call set anchor and that's when it would actually render the anchor. So that's why you see this huge gap where on the server, the whole page renders without the anchor then the bundle arrives, hydration happens and that's when you see it. So again, if it's just client side, it's not that big a deal but the moment we start rendering on the server, especially with slow networks, this could take a while for the bundle to arrive. So that's not great, that's not ideal, none of that. So how can we get this button to render sooner? How can we set the anchor on the server instead of the client? The answer is client. Sorry, so what we do here is we say the anchor is null, let's start with that in the action menu and then React has this top level API of react.children and it has a bunch of functions. So what you do is you give it props.children and then it lets you go through the child and inspect its properties. One of the properties a child has is child.type and child.type is really interesting because it gives you the function that was used to create this component. So in the case of action menu.button, it would give you the function that was used, which is this menu button, which means you can do things like this. You can say child.type and just use the reference to compare it. If child.type is menu button, that means you have action menu.button and then you can actually just say set the anchor. So I'm saying anchor is null, I'm mapping through if it's menu button, I'm just saying anchor is child. And if it's menu overlay, then let's clone that element and pass it an additional prop. Again, all of these are top level APIs and I'm passing the render anchor prop. I'm creating that on the fly and passing it to menu overlay. So this is super interesting because all of this has happened even before action menu hasn't done anything. So now, when even on slow 3G when I reload, you'd see that everything is coming from the server. This is already done. And this is an interesting pattern. And the constraint of here, of course, is that you have to use menu button or menu overlay, you can't use a third component. And we throw an error if you try, so it says action menu does not identify what you passed, please use none of these or refer to the documentation. And then add a nice comment for my co-workers because I'm not a monster. So, let me ask you, is this a hack? Absolutely. But is this a code crime? For all of us. So, a component that, let me put it this way, the goal of a good component, just like good UI design is to abstract away annoying words. Especially work that is not core to the goals of the user or the developer in this case.

So, sometimes crimes are okay, you know. And a better way to put that might be that hacks that made the code easier to read an author, helping the developers do a good job quicker within reasonable constraints of the system are kind of okay. So, the reasonable constraints here are that if you're using action menu, you have to use action menu.button and action menu.overlay, which works really well because those are also components from the same component library and we're using all of this to build GitHub. Maybe if you're an open source library like material or chakra, you might not be able to impose those constraints. People can bring their own button, but it works well for us.

So, moving on. Cool. Take a deep breath. Okay, more crimes. So, we have this component called a nav list or a navigation list. And the way this component works is that we have a nav list and you can render nav list or item inside it. So, we use this on the settings page where you have a bunch of components, a bunch of setting pages and then we add this item as a list. You can have an icon. You can have a gift.

So, for example, when I click this public profile, this is the selected item. And the way we show this selected state in the site then is by using the aria-current prop. So, in a list of, in a navigation list, it's good practice. It's like that makes the page accessible to screen reader where you say aria-current and then if this is the current page that is selected, then you say aria-current this page, otherwise you said nothing. So, we try to reuse this aria property to show the styles for the selected state as a way to nudge people or promote people to use accessible properties.

And of course, there's a bunch of these. The settings is like really long. GitHub is big. So, we have a bunch of them. It only makes sense to start grouping them in some way. So, creating this component called navlist.group. It's pretty similar. It takes a title. It takes an icon and then it takes, again, navlist.items. You can have navlist.items without any group or you can group them inside this and you get a nested navigation list.

So, what's interesting about this, again, is the way we show the IO current. We show the current selected item, navigation item, is by reading IO current from this list. So, when you reload this page, use the selected item would be highlighted. It's you too. But what's more interesting is, if it's part of a group, then this group would be open. So, if I try to open public profile, all the groups are closed, but if I reload on one of the billing implants, which is part of access, the group would be open by default, which makes sense, right? You want to know where the thing you are.

So, the group basically has to know if one of its items is the IO current value. So, how does this work?

Now, this is kind of boring. It's a navigation with a list inside it. And that item is also kind of boring. It's the list element inside, because the parent is a UL. So, this has to be an LI. And then we render a link component from the design from the component library. So, all the fun things are in that.

So, that group, of course, is the list element because it renders inside a UL. And this thing is a button, because you click it and you can change state. It has a meeting icon. We render children's next to it. And now we need a way to basically identify if this is open or closed. So, we set state, we say, we manage the state. The state can be toggled by the button. And then we can even do this carry it as a trailing icon. I carry it up or carry it down, right? Like this one.

So, all this is pretty cool when you're interacting with it. But what is the default value of this? We kind of need to know if this nav group has an item inside it, which has ARIA current, so that we can set that as default open is equal to true. Now, a reminder that this is what we're looking for, but we're looking, we kind of need to access this value up here on the group. So, one of the options could be just ask the user, right? Ask the developer who's using this component to set the default value. And that obviously works, but if you think about what the developer experience here would be, what kind of code they would have to author, it would probably be something like this, where you have to maintain a list of all of these, and then say if it includes any of them, then this should be open. And they're already doing part of this information here, and this feels a bit redundant. This obviously wouldn't work on SSR, so you probably would have to access like a router here. And it just feels like a lot of work that is in core to what they're trying to do, which also means that we can be missed and then you get bad user experience. So a smart component would just absorb this out.

So how do we send this value automatically? One way of course would be to do crimes to pick the components out of the children. And we've done something like this before, where we can say default open as false, and then we map through all of the children. And if any one of them has ARIA Apparent on them, then that default open should be true, right? That makes sense. And that's pretty easy.

But remember I said that apps that make code is allowable within reasonable constraints of the system are okay. Now, when we started testing out is this a valid constraint? Can navigation list... Sorry, can the NavList group assume that it will have this item and be able to pick ARIA Apparent is where we ran into problems. So very often people would use a different router, like they can use Next router or a React router or a Remix router. And then what they would end up with it is navigation list might have a custom component sided like Router link, and even inside that, the ARIA Apparent might not even be on NavList.item. It might be on something like Next link. So we kind of lose that. We can't just go into the component and read the prompts because that wouldn't be the case here. So those are not reasonable constraints in the system. We can't even rely on callbacks because as you can see here, NavList.item doesn't even know if it's selected because this is a property of next link now. We could change a few things. We could again go back to default open, but it would be really nice if this component could do it on its own. So if we can't assume children, how can we set the default value? The answer of course is Grand. So what we do here is the CreatorRef and then the attachment on the root element of the group. And after it has mounted, the imperative will say QuerySelector for ARIA current. And this is a fun thing because QuerySelector is happening on the rendered DOM. So it doesn't really care about how the components are structured. Is it a direct child? Is it deeply nested? None of that really matters because, well, in this case it's already rendered and it's somewhere inside the group. So we do a QuerySelector and we do setOpens. Of course, even, you know, it totally works here. So what's interesting about all of this is, like, is it an anti-pattern? It kind of is, right? When we imperative API calls with QuerySelector inside of the app component, which has a dexterity of design. So it's kind of an anti-pattern, but is it a code crime? Help me out. I think the goal of a good component is abstract away all this annoying work. And to figure out if a group has an item selected and open it, feels like this is not core to what the setting stage is. Right? You wanna focus more on what settings are you changing? What are the forms for the user experience? This feels like an extra perk. So I think it shouldn't be part of it. So, okay. This crime feels okay.

Okay, one for the road, because we're short on time. There's something, this is not perfect yet. If it's on client side, then it's fine. You know, it doesn't really matter. But if they're using SSR, then it kind of leads to this, where use layout effect would only happen on the client side, not on the server. It would happen after mount. So you end up with something like this, where it's closed on the server and then only opens in the client. Which kind of, I mean, don't get me wrong. This kind of looks cool. I like that this is like a tiny animation. So that works. But if it's on slower 3G, then it shouldn't work. Yep, this is good. So if it's on slower 3G, it would open after a really long while, right? Like it doesn't know yet the bundle arrives and only then it hydrates, runs use layout effect and opens. And this is a really long time. You might already be looking at some other setting and then suddenly opens and distracts you. So that's not really good. The answer, the hint I can give you is that it's obviously clients. And what we want is, we want the default value to be set sooner, right? We don't necessarily need this to happen on the server. We just want it to be sooner. And that's all the hint I'll give you.

And I'll leave you with this last part, which is acts that make the code easier to read and also helping developers do a good job within reasonable constraint with systems. So sometimes crimes are okay. Other times that's me, that's me on Twitter. I'll see you there. Sidd, it's always great to hang out with you. Unfortunately, you're not here, but I feel like we did this last year as well, joined virtually. It's good to see you again, my friend.

How are you doing? I'm doing good, how are you? I'm doing good. All right, let's jump into the questions. We have a question which ask, is it a good idea to implement a workaround that depends on the frameworks, internal APIs? And what if React decides to stop supporting that in the future? That's a good question. I'd say internal API is definitely not, but if it's a top level API like React.java, then you can get away with it. I'm going to be very honest, most of these are hacks, right? But sometimes hacks get the job done. And I feel at this point, with the way React team is committed to supporting backward compatibility, they're kind of arm-twisting that. They can't really break any of these top-level APIs. You know what, Sid? Next time someone asks me a tough question about is it a good idea to make that decision, I'm going to grab that slide. I'm going to quote you on it as well.

One thing, actually. A lot of people, I always love your presentation style and the way you show us live code and show us those demos. Someone's actually asked about that tool you use. What are you using to control the speed tests on examples? Because when we are building our own platforms, we want to sort of do these tests in different environments. What do you use? Right, so usually what I do is I use the Chrome network tab and that has multiple emulation. For the talk, I'm not doing any of that. That was a simulation. I did basically, I was like, please use this is how much speed I'm allowed. And then I would lower down my actual code. No problem, like we said, hacks are good if you can get away with them. But about hacks, do you think hacks like these can impact the readability of the code in the long run? Especially when you start finding things in places maybe you didn't expect to see them? Oh, definitely. So I think the big caveat I'd put here is that I wouldn't recommend these hacks if you only need them once, or if you can, you know, if it's an application man. The way I justify my hacks are some of these, somebody has to do them, right? If React doesn't have a top-level solution for this, you have to put that code somewhere. So I would rather put it in the library that gets reused and write a bunch of tests for it than put it closest to the user that I'm probably not writing as many tests.

So I'd say it affects the readability and also the maintenance. Like we write a bunch of tests for these crimes because you are playing with fire. Since you messed something up, you could cause a bunch of things to break. So it affects it, but I feel like you have to find where to put the complexity because it has to go somewhere. So put it where you can maintain it with the most confidence.

So one thing also, this is kind of my own question I'm throwing in here. One thing I love is every time I get to watch one of your talks I see something new. I've learned about design systems and things that you are clearly getting from seeing how developers are interacting with the design systems that you work on. How do you go getting that feedback and seeing the behaviors that they are building with and then trying to kind of implement maybe good practices from a design system perspective?

That's a good question. I think what they end up doing a lot is because they use GitHub to build GitHub and its meta, we just end up looking at patterns and there are patterns that get repeated a bunch. And in those patterns we can bring them into the design system ladder with doing Figma code, actually doing you know storybook, stories for pictures, and then we kind of start from the end. Then this is what we'd like our users, the developers that build GitHub to have. So what kind of abstraction do we need to build to support that? So I guess the first stage is inventory where you just look at all the things that exist and all the things that can be simplified or can be made into a meta pattern and then you start from there.

Thank you so much Sid. I know someone popped in a question right at the end. Unfortunately we don't have time right now, but if you head over to the speaker's Q&A room or if you're online and you're pitting online, go to the speaker's Q&A in Discord you can ask Sid and he can answer your question.