This post is about what I feel is a problem in the way some developers on the front-end are writing their components – whether in React, Polymer, or now Angular 2. Please feel free to disagree with me in the comments section, as I’d like to know if I’m missing something.

A Widely Agreed-Upon Worst Practice

jQuery brought a lot to the table when it hit v1.3. We were able to suddenly manipulate DOMs with ease, and we who spent our time on the front-end were suddenly burdened with such things as code readability. Concepts like separation of concerns were, at the time, way off our radar.

As such things generally go, as we worked with each other’s code we began to notice patterns that appealed to us – what made things easiler, what made things more readable, etc.

Take, for example, the following:

// example.js
(function() {
    var user = window.user || null,
        color = user ? '#428bca' : '#333',
        text = user ? 'Welcome!' : 'Sign in!';
    $('#welcome-tag').html('<h2 style="color:' + color + ';">' + text + '</h2>');

There’s a glaring no-no in this block above (as well as possibly one or two others) that would earn my ire if I saw it in a code review, which is the fact that it uses inline styles programmatically to paint HTML. If I had seen this in a code review, I would kick it back and tell the person who wrote it to put their styles in classes, add those classes app’s Sass files, and then try again.

Why we separate concerns

Scalability is probably my topmost concern when I write code. Embarassment holds a close second place. When I write, I wonder what someone coming into my project a year or two down the line will think when they open it up. Will they be able to navigate through the code easily enough? Or will they gather their buddies around to laugh/cry at the massive undertaking ahead of them?

One of the nicest things you can say about another developer is that their code is easy to read. — Oscar Pineda

Part of what eases my mind is knowing that my file structure generally makes a lot of sense. I separate my styles out to Sass files, my HTML out to template files, and my programming out to JS files. If you’re looking in a directive file and want to find the template that goes along with it, it very likely has the same name, and always lives in the same directory. If you’re wondering why a link is a certain color, you can open my project’s theme files and find the _links.scss file. Because that’s where you’d expect those styles to be.

We separate concerns for plenty of historic reasons, but among them – the front-end is friendly like that. Our styles don’t have to live within our HTML files or our JS files. They can live in theme files built specifically for Sass, LESS, or CSS. A change in styles means only changing what’s in a file built for handling styles; as opposed to, for example, a file that’s built for data dispay logic. And when someone opens the project for the first time, they don’t have to hunt down where a style might live. It doesn’t take hours of trudging through inline and programatic styles to find the right one.

Enter Components

React has been out for a while. Polymer v1 is about six months old (I think). Angular 2 just hit Beta. Component-based architecture is here to stay – at least for a while or until we find something better. One of the practices that seems to be emerging is this idea that the HTML, JS, and theming of a component should all live in the component file. This is a bad thing.

Think back to your days in Angular 1.x. Did you ever, or would you ever have programmed a directive that had its own style declarations written in the inline template? Something like this:

angular.module('demo', [])
    .directive('badDirective', function() {
        restrict: 'E',
        scope: {},
        template: `
                h2.welcome {color: #333;}
                h2.welcome.logged-in {color: #428bca;}
            <h2 class='welcome' ng-class={'logged-in' : user}>{{text}}</h2>`,
        link: function(scope) {
            // ... some scope stuff about users being logged in ...

You could have. There was never anything stopping you. But you didn’t, because reasons:

  • Inline styles promote the destruction of a consistent global theme
  • They’re never where you want them to be when you first come into a project
  • They break your pre-compile (Sass or LESS) workflow
  • They make portability more difficult by embedding specific implementation theming into something that should be abstract

(There’s a lot that could be said about Shadow DOM scoping styles to its own component and its component’s children. In the example above I specifically used classes to apply listed styles directly to elements in the directive template. This could have been accomplished a few different ways, but ultimately the example is specific to the directive template, regardless of how you write it.)

And yet, despite all of this, I came across the following block in a plunker linked from the Angular 2 developer’s guide:

// app/app.components.ts

    selector: 'my-app',
    template: `
        <a [routerLink]="['Dashboard']">Dashboard</a>
        <a [routerLink]="['Heroes']">Heroes</a>
    styles: [`
        a {padding: 5px;text-decoration: none;}
        a:visited, a:link {color: #444;}
        a:hover {color: white; background-color: #1171a3;}
        a.router-link-active {color: white; background-color: #52b9e9;}
    directives: [ROUTER_DIRECTIVES]
    // {path: '/', redirectTo: ['Dashboard'] },
    {path: '/dashboard', name: 'Dashboard', component: DashboardComponent, useAsDefault: true},
    {path: '/heroes', name: 'Heroes', component: HeroesComponent},
    {path: '/detail/:id', name: 'HeroDetail', component: HeroDetailComponent}
export class AppComponent {
    public title = 'Tour of Heroes';

There it is, right? Styles and programming in the same file. Additionally, I’ve seen this while sitting in React presentations, and I came across it several times when doing research for a Polymer PoC I put together this past fall.

To reiterate, this is a bad thing. It’s the code-review equivolent to seeing the jQuery-based block written at the top of this post, and is a practice we squashed a long time ago in jQuery apps, and in Angular 1.x. Again, for reasons:

  • Inline styles promote the destruction of a consistent global theme
  • They’re never where you want them to be when you first come into a project
  • They break your pre-compile (Sass or LESS) workflow
  • They make portability more difficult by embedding specific implementation theming into something that should be abstract

Normally I wouldn’t call out the Angular team, knowing full well how busy they are, and how much they make our lives easier. However they repeat this practice several times throughout their guides, including on a page with the following quote about best practices:

It’s only a tutorial but we can still do things right…

The irony is that throughout that very tutorial, there are dozens of references to avoiding hard-coded data, being sensitive to implementation changes, and abstracting for re-usability. All of these goals are violated when you add theming to your component.

The more theme styles we pack into our components, the more we play a Russian doll game of nesting styles within DOMs, assuming that they’re always applicable to the implementation we’re thinking of at the moment we write them. As a new developer comes onto the project with new stylistic requirements, they have to open up files to hunt down nested styles – hopping from one parent component to another as they traverse up or down through the component tree.

The Shadow DOM can/does inherit styles from the parent DOM. There’s nothing stopping anyone writing components from putting styles where they always would have gone in a larger application – in the global theme. Assuming that a component’s behavior depends on its styles grossly misunderstands the prupose of portability.

Comments are closed.