Skip to content

Instantly share code, notes, and snippets.

@Aaron-Pool
Last active June 29, 2018 17:20
Show Gist options
  • Save Aaron-Pool/c08841858b4f8a591a3fee2532ebcebc to your computer and use it in GitHub Desktop.
Save Aaron-Pool/c08841858b4f8a591a3fee2532ebcebc to your computer and use it in GitHub Desktop.

O Brave New World: Bringing Pubs in to Vuetopia

Well, we're finally to that point. It's finally time to start inching the pubs module toward the new frontier. But, in light of that exciting new opportunity, we have some immediate decisions to make. To integrate with Vue, we need webpack, and webpack also implies the pubs codebase being exposed to ECMA2015(ES6) features for the first time. So we need to decide if integrating new ES6 capabilities holds any value in simplifying and cleaning up the pubs codebase. Additionally, the general code-bundling system and philosophy we were operating under previously has done a lot to shape the way we were dealing with certain angular features, and we should take this opportunity to reevaluate many of those choices, so that they don't bog us down in the future.

That in mind, I've mostly organized our decisions into three distinct migration paths, which generally follow a linear progression from 'quick-n-dirty' to 'slow-but-thorough'. I've generally graded each migration path, somewhat objectively, based on:

1) time comittment

2) Possibility of introducing bugs into codebase

3) Long term maintenance outlook.

So, let's dive in, yeah?

Migration Choices

Here's a brief overview of each migration path. As you can see I've given each path a catchy name. Along with a summary of each possible migration path, I'm including a table here for quick reference/comparison of the pros and cons of each path.

Risk/Plan Bare Minimum Es6 is our Friend Listen to Smart People
Time Invested Low Medium Medium-High
Bug Risk Low Medium Medium-High
Long Term High Medium Low
Path: Bare Minimum

Minimally invasive for bare minimum webpack functionality. Low short term risk, due to the smallest possible immediate codebase change. Highest longterm risk because we're just band-aiding old code with a modern bundler.

Path: Es6 Is Our Friend

Implement webpack functionality, and use ES6 features where sensible and valuable to reduce code size and complexity (lose dependency injection). There's a slightly increased immediate risk factor, compared to the first option, because it will result in a non-trivial codebase change, but it also diminishes the long-term risk of stale, melting-pot-of-technology code base.

Path: Listen to Smart People

Follow existing ES6 Angularjs styleguide, could include moving code and renaming files. This introduces a high short term risk, because it will result in the most substantial code base modification of all migration paths, but it has the smallest possibility for long term maintenance problems or "gotchas", since we're following an established pattern that has been fully developed by a third party, rather than us feeling things out as we go. And, luckily, we've been making pretty good architectural decisions already (overall), so aligning with most modern style guides still isn't too drastic of a change

Example Files

I just hammered out some pseudo-codeish trivial files to demonstrate how each migration path would transform our code base. Here's the example files we'll work with:

Example Controller
(function() {
    'use strict';
    angular.module('pub').component('pubComponent', {
        template: [
            '$templateCache',
            function($templateCache) { return $templateCache.get('pubComponent.tpl.html'); }
        ],
        controller: pubComponentCtrl,
        bindings: { _someBinding: '<someBinding'},
        require: { ...component parent requirements}
    });
    
    /*@ngInject*/
    function pubComponentCtrl(...dependency injections) {
        var $ctrl = this;
    
        /* ctrl attributes */
        $ctrl.someAttribute = null;
        $ctrl.otherAttribute = null;
    
        /* ctrl api */
        $ctrl.someMethod = someMethod;
        $ctrl.otherMethod = otherMethod;
    
        /* ctrl lifecycle */
        $ctrl.$onInit = componentHelperService.onInit($ctrl);
        $ctrl.$onChanges = componentHelperService.onChanges($ctrl);
        $ctrl.$postLink  = componentHelperService.postLink($element);
        $ctrl.$onDestroy = componentHelperService.onDestroy();
    
        /* ctrl api definition */
        function someMethod() { /* .. some method code */ }
        function otherMethod() { /* .. other method code */ }
    
        /* ctrl private functions */
        function somePrivateFunction() { /* .. some private function code */ }
    }
}());
Example Service
(function() {
    'use strict';
    angular.module('pub').service('pubService', pubService);
    
    /*@ngInject*/
    function pubService(...dependency injections) {
        var service = this;
        /* service api */
        service.someMethod = someMethod;
        service.otherMethod = otherMethod;
        
        /* service api definition */
        function someMethod() { /* .. some method code */ }
        function otherMethod() { /* .. other method code */ }
    
        /* service private functions */
        function somePrivateFunction() { /* .. some private function code */ }
    }
}());
Minimally Invasive Procedure
Example Controller Transformation

The controller definition in this migration scheme remains essentially the same, except a function is exported from the file, which, when provided with a module from the caller, registers itself with that module. Each component and service would the be imported into a module file (or a parent index file, if more granular grouping is needing as the code base gets large) and registered by running the imported function. The example files would then look like this:

import template from './pubInformation.html';
import style from './pubInformation.less';
export default {
    register(module) {
        module.component('pubInformation', {
            template: template,
            controller: pubComponentCtrl,
            bindings: { _someBinding: '<someBinding'},
            require: { ...component parent requirements}
        });
    }
}
        
/*@ngInject*/
function pubComponentCtrl(...dependency injections) {
    var $ctrl = this;

    /* ctrl attributes */
    $ctrl.someAttribute = null;
    $ctrl.otherAttribute = null;

    /* ctrl api */
    $ctrl.someMethod = someMethod;
    $ctrl.otherMethod = otherMethod;

    /* ctrl lifecycle */
    $ctrl.$onInit = componentHelperService.onInit($ctrl);
    $ctrl.$onChanges = componentHelperService.onChanges($ctrl);
    $ctrl.$postLink  = componentHelperService.postLink($element);
    $ctrl.$onDestroy = componentHelperService.onDestroy();

    /* ctrl api definition */
    function someMethod() { /* .. some method code */ }
    function otherMethod() { /* .. other method code */ }

    /* ctrl private functions */
    function somePrivateFunction() { /* .. some private function code */ }
}
Example Service Transformation

Again, a simple transformation that's very similar to the way controllers are transformed.

export default {
    register(module) {
        module.service('pubService', pubService);
    }
}
    
/*@ngInject*/
function pubService(...dependency injections) {
    var service = this;
    
    /* service api */
    service.someMethod = someMethod;
    service.otherMethod = otherMethod;
    
    /* service api definition */
    function someMethod() { /* .. some method code */ }
    function otherMethod() { /* .. other method code */ }

    /* service private functions */
    function somePrivateFunction() { /* .. some private function code */ }
}

And supposing you had a file structure like this somewhere in the codebase:

-pubCommunication
   |-pubCommunication.js
   |-pubCommunication.less
   |-pubCommunication.tpl.html
   |-pubCommunicationHeader
       |-pubCommunicationHeader.js
       |-pubCommuncationHeader.less
       |-pubCommuncationHeader.tpl.html
   |-pubCommunicationStep
       |-pubCommunicationStep.js
       |-pubCommuncationStep.less
       |-pubCommuncationStep.tpl.html
   |-pubCommunicationNavigation
       |-pubCommunicationNavigation.js
       |-pubCommuncationNavigation.less
       |-pubCommuncationNavigation.tpl.html

You could add an index.js file to pubCommunication with the following contents:

import header from './pubCommunicationHeader/pubCommunicationHeader';
import step from './pubCommunicationStep/pubCommunicationStep';
import navigation from './pubCommunicationNavigation/pubCommunicationNavigation';

export default {
    register(module) {
        header.register(module);
        step.register(module);
        navigation.register(module);
    }
};

That way a file could simply import communication from './comm/relative/file/path'; and register all communication and communication sub components by calling communication.register(module).

And you could easily continue this pattern all the way up to the root module folder where the actual module file (e.g. pub.js) might look like:

import components from './ui/components';
import services from './ui/services';
import config from './ui/configs';

export default {
    initialize() {
        let pub = angular.module('pub', [
            'someDependency',
            'someOtherDependency'
        ]);
        
        // register all ui parts
        components.register(pub);
        services.register(pub);
        config.register(pub);
    }
}

So the overall migration steps are:

  1. Export registration function (expects to be provided with a module)
  2. Group components/services/etc logically using folder structure and index files
  3. Create the module in the root module javascript file and provide it to the registration function for all top-most parent registration functions to add all component/services/etc to the created module

The advantages of this approach is that, as stated, it's minimally invasive. It would also probably be the quickest way to get things in a workable state. The disadvantages of this approach are that it's the least modern of the possible approaches, and it will, more than likely, present the greatest possible disparity with future code we write. Its essentially a fresh coat of webpack paint on a slightly rusting application, so it will be prone to age quickly.

Webpack + true ES6 migration
Example Controller Migration

This involves transforming the simple controller function into a full fledged class, and also replacing angular dependency injection with es6 built in modules (with the exception of a handful of dependency injections that angular populates based on the calling context, e.g. controller injections $element, $scope, and $attrs, which have component specific content dependent on dom information about thed component). Also, we would/could/should write a base class resComponentController that would handle some of the boiler plate work we have in our current components (for example, it would automatically run our lifecycle hooks through the componentHelperService), just to clean things up a bit. So this migration path leads to a controller that looks like the following:

import template from './pubInformation.html';
const component = {
    template,
    bindings: { 
        _someBinding: '<someBinding'
    },
    require: /* controller requirements here */,
    controller: class controller extends resComponentController {
        // these would be the only things ever injected
        constructor($scope, $element, $attrs) {
            super($scope, $element, $attrs);
            // attribute declarations
            this.someAttribute = null;
            this.otherAttribute = null;
        }
        
        /* Life Cycle */
        $onInit() {}
        $onChanges() {}
        $postLink() {}
        $destroy() {}
        
        /* API */
        someMethod() { /* .. some method code */ }
        otherMethod() { /* .. other method code */ }
    }
}

/* private functions */
function someMethod() { /* .. some private function code */ }
export default {
    register(module) { module.component('pubInformation', component); }
}
Example Service Migration

In this migration path, services break out of the angular framework completely, as dependency injection is no longer needed (there are no instance-specific dependency injections for services, unlike components). So the service would transform something like this:

import {$http} from 'ngImport';
import utils from '@shared/services/utils';

export default class pubService {
    /* service api definition */
    static someMethod() { /* .. some method code */ }
    static otherMethod() { /* .. other method code */ }
}

/* service private functions */
function somePrivateFunction() { /* .. some private function code */ }

Additionally, we could to quick pass-through to simplify code with new es6 features, like default variables, template literals, spread operator, enhanced object literals, and arrow functions.

So, the overall migration steps would be a superset for option 1.

  1. Convert services to simple class singletons with static methods, convert service dependencies in to simply imports
  2. Convert component controllers to class annotation. Replace dependency injection with simple imports (except for $scope, $attrs, and $element).
  3. Export registration function
  4. Group components/services/etc logically using folder structure and index.js files (purely for the purpose of clearer hierarchy and code organization)
  5. Create the module in the root module javascript file and provide it to the registration function for all top-most parent registration functions to add all component/services/etc to the created module
Modern Angular ES6 Style guide

I could create examples of this approach, but it would be much simpler to just link to the styleguide. The examples are much more comprehensive than my little fake files: https://github.com/toddmotto/angularjs-styleguide.

The only exception to this is that, in my opinion, we should still abandon dependency injection. Todd continues using dependency injection because, I think, he's gearing his guide toward people who will eventually migrate to Angular 2 or 4 or 6 or 2000 or WHATEVER. And even the new Angular framework uses dependency injection. Our target migration framework (vue), however, does not. So abandoning dependency injection will bring us into closer parity with future code. It will also make auto complete and code suggestions for editors potentially much more accurate. Dependency injection is basically magical as far as intellisense goes, so Visual studios really struggles to try to make accurate code completion suggestions.

One thing worth pointing out is that the more granular module definitions he suggests make several things (or at least one thing) we currently struggle with much cleaner/easier to understand. Specifically, state configuration and routing could be moved directly beside the component that uses that routing, instead of floating around in a config folder, like we have now.

The over all migration path would be similar to step 2, but with a few important deviations shown below:

  1. Convert services to simple class singletons with static methods, convert service dependencies in to simple imports (Same as option 2)
  2. Convert component controllers to class annotation. Replace dependency injection with simple imports (except for $scope, $attrs, and $element). (Same as option 2)
  3. Export registration function Export actual component/directive/config object/function.
  4. Group components/services/etc logically using folder structure and index.js files (purely for the purpose of clearer hierarchy and code organization) using angularjs's existing module system to import and logically group ui component/service/etc, then export those more granular modules to be imported and required by parent modules.
  5. Create the actual angularjs module in the root module javascript file and provide it to the registration function for all top-most parent registration functions require declared submodules to add all component/services/etc to the created module.
  6. Rename files based on their type *.module, *.template, *.component, *.service, etc
A few final considerations:
  • Now would be a good time address any refactors or changes we want to see in our current codebase.
    • We're going to have to regression test all of pubs, essentially, to migrate to webpack anyways, makes this period a good candidate for making other significant changes, if we want.
    • The way I see it, current code is going to further calcify once we move to the Vue world.
      • Wouldn't mind refactoring a lot of the complexity that's shown up in the "config" folder.
      • Implement *Helper pattern anywhere it's missing
        • Abstracting logic heavy logic into a helper service injected into the controller, instead of being in the controller itself has been a pattern I've really liked and has made controllers much easier to understand and maintain, in my opinion
        • I'd like ti add that accross the board, particularly to controllers that I've written to a point that they're massively bloated (e.g. pubCommunicationCurrentStep)
      • Identify high impact code that was written a while ago and could probably be substantially improved, performance wise
        • Highlight indicator (the thing that hovers and animates between active menu items in various places throughout the app is really render intensive, and I know of a pretty easy way to improve the performance substantially
        • The communication progress header was one of the first things I wrote, and could probably be easily and drastically improved with some TLC, could probably even add some fancy animations.
        • I could probably even further improve the performance of the card screen, if that's something we're interested in.
      • Simplify the communication process code and data model
        • I would love it if we could set up a SPROC and end point that loads and writes an entire communication object. A lot of the bloat in the code for everything below pubCommunication would go away if we had a simpler load/save option
  • Do we want to upgrade any of our dependencies?
    • as we discussed earlier, do we want to take this opportunity to upgrade ui-router. It would remove a lot of the state complexity we currently have in lazy loading code and component routing, since lazy loading and component routing are both supported by default in most recent versions of ui-router.
    • Do we want to upgrade to the final release of angularjs (1.7) ?
    • Upgrade pdf viewer? The set up we have right now is pretty convoluted, and could be vastly simplified.
Tech Debt Retrospective

Here's a comprehensive list of non-webpack-migration things we should consider addressing

  • Low Risk
    • Pdf Viewer
    • Lower visibility performance improvements (e.g. highlight-indicator)
  • Medium Risk
    • UIRouter
    • Angular 1.7
    • High visibility code performance improvements that are generally contained to a small area of the app (communication process header, card screen)
  • High Risk
    • Any high level refactors (communication process, config set up, etc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment