Refactoring Vue: Cleaning Up a List of Posts With Better Component Splitting and More ES6

What's this all about?

I used to write Vue.js. In 2015 I did a series on Twitch/YouTube where I learned Vue "out loud", sharing my (often-painful) process learning Vue using the very minimal amount of material that was available at that point.

I've written some Vue since 2015, but I've also learned some React, written a lot of Laravel, run a company, and spent much of my free time writing a book about Laravel.

It's time for me to get back into Vue.js and really spend some time to get good at it. Thankfully, some of the best Vue developers out there work at Tighten, so I'm putting them to work to level me up.

So, I'm going to be writing new Vue code and also cleaning up some of my Vue from 2015, and I wanted to share the process with you, my lovely readers.

What are we looking at today?

This is a little internal tool I'm building named PostIt. It helps me and the rest of the team remember to submit all of our new content (blog posts, etc.) to any content aggregators ("targets") that don't have APIs available.

I built the original proof-of-concept on a plane in about two hours, with no access to the Vue documentation or sample apps to look at, so it's the perfect opportunity for a refactor. I left a lot of @todos laying around the code and couldn't cheat and look anything up.

What did the code originally look like?

It's a lot. Here's basically what happens in the code below:

1. DashboardController pulls in all the "targets" (right now, that's just Laravel News). It also pulls in all of the posts we've had recently, grouped by source (e.g. Tighten Blog, Mattstauffer.com).
2. dashboard.blade.php JSON-encodes those and passes them to Posts.vue.
3. Posts.vue starts a table and iterates over the sources. For each it shows a header and then iterates over the posts for that source, passing each into Post.vue.
4. Post.vue shows a line for each post, and then a checkbox for every target. That checkbox reflects whether or not there's already a "submission" record for that post--basically, whether or not that post should be checked off for that target.
5. If the user toggles the checkbox, the underlying data model is not updated... but it works anyway because checkboxes have their own native state tracking; and then an HTTP call is made to create or delete the appropriate submission.

Note: this is a lot of code, and it was a bit hard to read, so I removed all the styles to make it a bit cleaner.

DashboardController

// For non-Laravel developers, this just passes a list of all the targets as $targets
// and a list of all the sources as $sources, each with a $posts relationship filled,
// and each $post with a $submissions relationship filled
return view('dashboard')
    ->with('targets', Target::all())
    ->with('sources', Source::with(['posts', 'posts.submissions'])->get());

dashboard.blade.php (the 'dashboard' view referenced above)

<Posts :targets="{{ json_encode($targets) }}" :sources="{{ json_encode($sources) }}"/>

app.js

require('./bootstrap'); // Laravel bootstrap code

window.Vue = require('vue');

Vue.component('Post', require('./components/Post.vue'));
Vue.component('Posts', require('./components/Posts.vue'));

const app = new Vue({
    el: '#app'
});

Posts.vue

<template>
    <table>
        <tbody v-for="source in sources" source="source" targets="targets">
            <tr>
                <td>{{ source.name }}</td>
                <td></td>
                <th v-for="target in targets" class="pr-4 text-sm">
                    <a :href="target.url" target="_blank">{{ target.name }}</a>
                </th>
            </tr>
            <Post v-for="post in limitPosts(source)" :targets="targets" :post="post" :key="post.id" />
        </tbody>
    </table>
</template>

<script>
    export default {
        props: [
            'sources',
            'targets'
        ],

        methods: {
            limitPosts(source) {
                return _.slice(source.posts, 0, 5);
            }
        },
    }
</script>

Post.vue

<template>
    <tr>
        <td>
            <a :href="this.post.guid">{{ this.post.title }}</a>
        </td>
        <td>
            {{ this.post.published_at }}
        </td>
        <td v-for="target in this.targets">
            <input type="checkbox"
                :checked="submittedToTarget(target)"
                @click="toggleSubmission(target, submittedToTarget(target))"
                />
        </td>
    </tr>
</template>

<script>
    import axios from 'axios';

    export default {
        props: [
            'post',
            'targets'
        ],

        data() {
            return {
                submissions: []
            };
        },

        mounted() {
            this.submissions = _.map(this.post.submissions, (submission) => {
                return submission.target_id;
            });
        },

        methods: {
            // @todo can this be a computed prop instead?
            submittedToTarget(target) {
                // @todo there's gotta be a cleaner way
                return _.filter(this.submissions, (submission_target_id) => {
                    return submission_target_id == target.id;
                }).length > 0;
            },
            toggleSubmission(target, is_submitted) {
                const data = {
                    'target_id': target.id,
                    'post_id': this.post.id
                };

                // @todo cleaner way to make this not a conditional?
                // @todo There seems to be the need for a Vue-reactive way to modify this array?
                if (is_submitted) {
                    _.remove(this.submissions, target.id);
                    axios.delete('/api/submissions', { params: data });
                } else {
                    this.submissions.push(target.id);
                    axios.post('/api/submissions', data);
                }
            }
        },
    }
</script>

I knew some of the filter and map operations I wrote were wrong; the original version of this had a much more complex data model, and as I simplified it I knew I could find simpler collection operations. You can also see some @todos in there where I knew what I needed to do but not how.

When I got off the plane, I pushed up this code and then disappeared to take care of my family.

What refactors were suggested?

Two Tighten developers shared refactors for me.

Daniel Coulbourne: ES6 and collection methods

First, Daniel suggested a few high-level syntax changes. He pointed out I could change this:

// @todo there's gotta be a cleaner way
return _.filter(this.submissions, (submission_target_id) => {
    return submission_target_id == target.id;
}).length > 0;

to this:

return !! this.submissions.find(targetId => targetId == target.id);

Like I mentioned before, this was one of those places where I had simplified the data model, so I was already aware it needed to be better. But man, if there's ever been a good case for the beauty of fat arrow functions and collection methods...

He also pointed out that this:

this.submissions = _.map(this.post.submissions, (submission) => {
    return submission.target_id;
});

could be this:

this.submissions = this.post.submissions.map(submission => submission.target_id);

Beautiful.

Keith Damiani: the idiomatic-Vue refactor

Keith, one of our senior developers who is also fully responsible for building the beautiful and jealousy-inducing Tighten Typing Challenge, gave me a full refactor. I'll show his code, and then point out a few big changes he made.

dashboard.blade.php

<Dashboard :targets='@json($targets)' :sources='@json($sources)'/>

app.js

require('./bootstrap');

window.Vue = require('vue');
window.axios = require('axios');
Vue.config.productionTip = false;

import Dashboard from './components/Dashboard.vue';

const app = new Vue({
    components: {
        Dashboard,
    },
}).$mount('#app');

Dashboard.vue

<template>
    <table>
        <PostList v-for="source in sources" :source="source" :targets="targets" :key="source.id"/>
    </table>
</template>

<script>
import PostList from './PostList.vue';

export default {
    components: {
        PostList,
    },

    props: {
        sources: {},
        targets: {},
    },
}
</script>

~~Posts.vue~~ PostList.vue

<template>
    <tbody>
        <tr>
            <td>{{ source.name }}</td>

            <td></td>

            <th v-for="target in targets">
                <a :href="target.url">{{ target.name }}</a>
            </th>
        </tr>

        <PostItem v-for="post in recent_posts" :targets="targets" :post="post" :key="post.id" />
    </tbody>
</template>

<script>
import PostItem from './PostItem.vue';

export default {
    components: {
        PostItem,
    },

    props: {
        source: {},
        targets: {},
    },

    computed: {
        recent_posts() {
            return this.source.posts.slice(0, 5);
        }
    },
}
</script>

~~Post.vue~~ PostItem.vue

<template>
    <tr>
        <td>
            <a :href="post.guid">{{ this.post.title }}</a>
        </td>

        <td>
            {{ this.post.published_at }}
        </td>

        <td v-for="target in targets">
            <PostItemSubmission
                :submission="getSubmissionForTarget(target)"
                :post_id="post.id"
                :target_id="target.id"
            />
        </td>
    </tr>
</template>

<script>
import PostItemSubmission from './PostItemSubmission.vue';

export default {
    components: {
        PostItemSubmission,
    },

    props: {
        post: {},
        targets: {},
    },

    methods: {
        getSubmissionForTarget(target) {
            return this.post.submissions.find((submission) => submission.target_id == target.id )
        },
    },
}
</script>

PostItemSubmission.vue

<template>
    <input type="checkbox" v-model="has_submission"/>
</template>

<script>
export default {
    props: {
        submission: {},
        post_id: {},
        target_id: {},
    },

    data() {
        return {
            has_submission: Boolean(this.submission),
            url: '/api/submissions',
        }
    },

    computed: {
        payload() {
            return {
                target_id: this.target_id,
                post_id: this.post_id
            }
        },
    },

    watch: {
        has_submission(val) {
            if (val) {
                axios.post(this.url, this.payload);
            } else {
                axios.delete(this.url, { params: this.payload });
            }
        },
    },
}
</script>

Walking through Keith's refactors

Let's look at each of Keith's refactors one-by-one.

Renaming

First, Keith renamed Posts to PostList because it's both clearer and it also follows the Vue naming convention of two-or-more-word component names.

He then pulled out a wrapper for the Post list named Dashboard (which doesn't actually meet that naming convention, but, hell, it's the right name).

He renamed Post to PostItem for the same reasons.

Changes to bootstrapping

As you can see from my comments inline, the main changes here are to take advantage of ES6 imports and to update the construction of the core Vue instance to be more parallel in shape to the components we use elsewhere. The biggest wins here are consistency and therefore predictability and ease of onboarding new devs.

// Store axios on the window (global) since we'll use it all the time
window.axios = require('axios');

// Disable the Vue console log about building to production
Vue.config.productionTip = false;

// Use ES6 imports instead of `require`
import Dashboard from './components/Dashboard.vue';

const app = new Vue({
    // Register components when constructing, which is more parallel
    // to how we register components in other components
    components: {
        Dashboard,
    },

// Mount to #app after the fact, making the core Vue registration shape
// more parallel to how we register other components
}).$mount('#app');

Computed properties

In my original Posts.vue I looped over Post.vue using a method limitPosts, which grabs only the five most recents posts from the given source.

<Post v-for="post in limitPosts(source)" :targets="targets" :post="post" :key="post.id" />

What I should've considered is that since the Posts.vue component only has a single source, that's a perfect fit for a computed property--which is better than a method because its results get cached and only re-computed when its dependencies change. So, we moved from this:

    export default {
        // ...
        methods: {
            limitPosts(source) {
                return _.slice(source.posts, 0, 5);
            }
        },
    }

to this:

    export default {
        // ...
        computed: {
            recent_posts() {
                return this.source.posts.slice(0, 5);
            }
        },
    }

Which you can use like this:

<PostItem v-for="post in recent_posts" :targets="targets" :post="post" :key="post.id" />

You can also notice that he took advantage of ES6's collection methods--I'm an old head who still reaches for lodash for everything.

Importing components when they're needed

I unthinkingly registered my child component (Post) in the bootstrap:

require('./bootstrap'); // Laravel bootstrap code

window.Vue = require('vue');

Vue.component('Post', require('./components/Post.vue'));
Vue.component('Posts', require('./components/Posts.vue'));

const app = new Vue({
    el: '#app'
});

But, of course, that's not neceessary, because Post was never going to be used in my HTML, only within a child component. Keith fixed that by importing each component within the component that needs it:

<script>
// Inside of Dashboard.vue, importing and registering the PostList.vue component
import PostList from './PostList.vue';

export default {
    components: {
        PostList,
    },
}
</script>

More components!

One of the clumsiest parts of my solution was how I was trying to handle the representation of the checkboxes (targets) in the Post component. I knew something was wrong; I sprinkled @todos about how I wanted to handle them. I awkwardly updated the data model to simplify how I was representing their state ("submission"):

mounted() {
    this.submissions = _.map(this.post.submissions, (submission) => {
        return submission.target_id;
    });
},

But in the end, it just didn't feel right. I was having to create methods for toggling and checking state and everything felt way too dirty.

Turns out the answer was to move all of that onto a specific component for each checkbox. I had considered it but felt like it was going to be overkill, so I'm so glad Keith did it--it made all of the logic so much cleaner. That means PostItem.vue has almost no logic--just a single method to get the right submission for the target you're passing to the checkbox:

<template>
    <tr>
        <!-- // The rest of the row -->
        <td v-for="target in targets">
            <PostItemSubmission
                :submission="getSubmissionForTarget(target)"
                :post_id="post.id"
                :target_id="target.id"
            />
        </td>
    </tr>
</template>
<script>
import PostItemSubmission from './PostItemSubmission.vue';

export default {
    // ...
    methods: {
        getSubmissionForTarget(target) {
            return this.post.submissions.find((submission) => submission.target_id == target.id )
        },
    },
}
</script>

Now we have a new component: PostItemSubmission. Again, this is a checkbox that shows the target (Laravel News) for this source (Tighten.co Blog) and shows whether it's checked yet (whether or not a submission exists yet). Its logic is a lot simpler; has_submission initializes its state by just checking whether a given Submission exists yet, but it's also bound to the checkbox via v-model and Keith is watching that value and when it's updated by the user, it's triggering axios calls to the server:

<template>
    <input type="checkbox"
        v-model="has_submission"
        class="my-2"
        style="transform: scale(1.25)"
    />
</template>

<script>
export default {
    // ...
    data() {
        return {
            has_submission: Boolean(this.submission),
            url: '/api/submissions',
        }
    },

    watch: {
        has_submission(val) {
            if (val) {
                axios.post(this.url, this.payload);
            } else {
                axios.delete(this.url, { params: this.payload });
            }
        },
    },
}
</script>

That's all for now!

Thanks for checking this out. Hopefully this won't be the last time I write some crappy Vue and get smarter folks to fix it up for me, publicly. Is there anything else you'd change? Hit me up on Twitter.