A core principle I keep in mind when creating a pull request is that I assume that the next developer to come along is going to continue doing what they already see here.

Switch like statements

For example, say I've written some code that looks like this:

type Product = {
    productType: "movie" | "book";
}

function ProductSuggestion(product: Product) {
    if(product.productType ==="movie") {
        return <div> 
            <img src="/movie-icon.svg"/> Enjoy a visual spectacle with... 
        </div>;
    }

    if(product.productType === "book") {
        return <div> 
            <img src="/book-icon.svg"> Jump into a great read with... 
        </div>
    }
}

Then we can assume that the next PR will look like this:

type Product = {
-    productType: "movie" | "book";
+    productType: "movie" | "book" | "album";
}

function ProductSuggestion(product: Product) {
    if(product.productType ==="movie") {
        return <div> 
            <img src="/movie-icon.svg"/> Enjoy a visual spectacle with... 
        </div>;
    }

    if(product.productType === "book") {
        return <div> 
            <img src="/book-icon.svg"> Jump into a great read with... 
        </div>
    }

+    if(product.productType === "album") {
+        return <div> 
+            <img src="/album-icon.svg"> Rock out with... 
+        </div>
    }
}

Whereas if our code look liked this:

type Product = {
    productType: "movie" | "book";
}

const productToContentMap : Record<Product['productType'], {
    image: string, 
    description: string
}> = {
    'movie': {
        image: "/movie-icon.svg", 
        description: "Enjoy a visual spectacle with..."
    }, 
    "book": {
        image: "/book-icon.svg", 
        description: "Jump into a great read with... "
    }
}

function ProductSuggestion(product: Product) {

    const content = productToContentMap[product.type];
        return <div> 
            <img src={content.image}/> {content.description}
        </div>;
}

then we we can expect that the next PR will look like:

const productToContentMap : Record<Product['productType'], {
    image: string, 
    description: string
}> = {
    'movie': {
        image: "/movie-icon.svg", 
        description: "Enjoy a visual spectacle with..."
    }, 
    "book": {
        image: "/book-icon.svg", 
        description: "Jump into a great read with... "
    }
+    "album": {
+        image: "/album-icon.svg", 
+        description: "Rock out with... "
+    }
}

This isn't to say that, in this case, one style is better than the other, but it demonstrates that we can predict to some extent how a codebase is going to evolve given what we write now.

Tests

In the above scenario, this is quite a simple component, it might be tempting to skip the test.

If we do, that bit of friction in setting up the test boilerplate might also have the next developer also not writing a test.

Whereas if we write a single test:

describe(ProductSuggestion, () => {
    it("renders the product suggestion for movies", () => {
        render(<ProductSuggestion product={{productType:"movie"}}/>); 

        expect(screen.getByText("Enjoy a visual spectacle with...")).toBeInTheDocument();
    })
})

Then this will make it that bit easier for the next developer to write a test.

This suggests a corollary:

At least write one test.

This is a pragmatic kind of compromise, that doesn't require us to comprehensively test every line of code, but if the need arises, we are well placed to write a test.

This can be easier said than done! For example if our component has hooks into state management, then in order to just render the component, we also need to provide the state management. I write more about this problem here.. Writing one test then, actually serves a very useful purpose - it may sound like a low bar, but it's actually a very important one - if we can't clear that bar - then we might want to rethink things.

Questions? Comments? Criticisms? Get in the comments! 👇