Refactoring Am I Rent Stabilized

Motivation

Am I Rent Stabilized? continues to be a successful project; according to Google Analytics it still gets around 1,200 views and (double that for “impressions”) a month! However the last time I significantly touched the code for AIRS was over five years ago, back when I was still a fledging programer and web developer. Not surprisingly, I more recently found the code difficult to reason about, making it hard to add new features, fix bugs, or make necessary improvements such as those relating to web accessibility. I realized then, that if this project were to have a chance of living on, it would need a significant refactor. In this post I outline how I refactored AIRS; the goals and non-goals I set for myself, decisions I made along the way, and what I learned from it.

Part of what motivated me to do this refactor was reading Martin Fowler’s infamous book, Refactoring, Improving the Design of Existing Code. MF’s book not only provides a wide range of strategies for refactoring code, but also does a good job at justifying why refactoring is (or for many teams should be) a central part of software development. Refactoring the source code for AIRS offered a different set of challenges than my current day job as a UX Engineer where I tend to focus on prototyping UIs and data visualizations for the web. Though my prototyping work often acheives a high level of complexity, it is generally considered to be “throw away code”, meaning that it never sees the light of day in the actual product. The goal of prototyping is typically to validate or invalidate a hypothesis made by UX designers, UX researchers, and project managers. Thus the implementation of a prototype is not what is important; the learnings from it or the stakeholder buy in it helps generate is.

I think there’s something to be said about the importance of maintaining successful projects, or at least ones you dearly care about, versus sticking with only doing new projects on the side. I have not worked on a side project as the sole contributor in a good number of years, so I saw the appeal in doing things how I saw fit without having to spend time discussing and debating decisions with other contributors or team members. Even though side projects are still work, this can make the work a bit more enjoyable and a relief from the occasional tension and conflict inherent in team work.

Goals

Here are the goals of the refactor I decided upon:

1. Make the code easier to reason about by improving its organization and structure
   • use the common src/ directory approach with sub directories for related pieces, e.g. components/, utils/, actions/, reducers/, etc.
   • reduce code bloat by aiming to keep classes and functions small (say under 300 lines)
   • aim for being DRY but try not to preemptively optimize for this. To me this means waiting to abstract code after it’s apparent there’s duplication, instead of trying to abstract it right away when writing it.
2. Improve code quality:
   • use ES6 syntax
   • use ES modules
   • use ESLint for JavaScript linting
   • use StyleLint for (S)CSS linting
   • use Prettier for code formatting
   • add a commit hook that runs both Prettier and ESLint so that these tools run automatically
   • write unit tests using Jest
   • achieve good test coverage (>= 75%)
3. Remove unnecessary 3rd party JS dependencies (e.g. jQuery, CartoDB.js, Leaflet, etc.)
   • many of these aren’t absolutely needed for what the app does
   • some can be replaced using native browser features (e.g. fetch) or smaller 3rd party libraries (e.g. d3-geo)
   • although optimization isn’t a goal, by reducing the amount of 3rd party JS the site should load faster for users on mobile and slow internet connections
4. Update the frontend build system
   • the Gulp file and usage of Browserify was a bit messy and not easy to reason about, but maybe that’s just because I’ve become so used to working with Webpack
   • Use Webpack as a build system with Babel for transpiling ES6+ to ES5, code splitting, code minification, source maps, Sass, etc.
5. Have Continuous Integration and automatic deploys integrated with Github
   • Use Netlify for deploys and preview deploys for pull requests
   • Set up a Github Action for a CI build on pull requests that also runs the unit tests

Non-Goals

• Port the code to a JavaScript framework. This felt like the refactoring effort would have instead become a complete rewrite of the existing code. Again, the primary goal was to untangle the existing code to make it more easy to reason about, not to use the latest, greatest, shiny tech. Besides, out of the four pages that make up the site, only the main page requires much JavaScript, the other three pages are essentially static content and don’t require handling things such as managing application state and data flow.

• Port the code to TypeScript. As much as I’ve enjoyed working with TypeScript lately, it felt like doing so would be adding more complexity to the refactoring effort than was needed. TypeScript can always be added incrementally later if I chose to do so.

• Solve all the accessibility problems. There are quite a few of them that I intend to fix, and when it wasn’t too much effort to fix one while refactoring I went ahead and did it. I intend to file issues for the more significant problems and fix them later after the refactor.

• Optimize for performance. In MF’s Refactoring, he talks about how prematurely optimizing for performance can really hurt the readability of code and thus the ability for humans to reason with it. He suggests focusing on the refactoring first, then finding out where performance bottlenecks may be occurring and to address them when necessary.

• Focus on adding new features. I don’t really have any new features to add at this point anyway, so not a problem. I did end up adding autosuggest to the address search though, so I guess that counts as a new feature!

• Make it an opportunity to use a new fancy shiny piece of web tech. See my note above for why I didn’t port the code to a JavaScript framework like React, Svelte, or Vue.

Decisions

Perhaps most importantly, I chose not to use a JavaScript framework. I decided that adding a framework would be overhead that would get in the way of untangling and refactoring the existing code, which was written using a combination of jQuery and the revealing module pattern in ES5.

Ultimately I decided to:

• Keep the existing HandlebarsJS templates because it’s how localization / translations were implemented. I decided it would have added more work to port the Handlebars templates to regular HTML and use a JavaScript library such as i18next for translations without the help of a JS framework. That being said, I may end up doing this later as allowing for the page load without relying on JavaScript has very tangible benefits.

• Break up the translation JSON files by page name and two letter language code to make them easier to reason about. Previously there was one JSON locale file per HTML page, and each file contained translations for all three languages. Breaking them up makes them easier to edit and compare.

• Organize the code methodically and consistently following the component pattern to decouple features using ES6 classes. Each interactive element on the page received its own Component class, and inherited from a parent class so that components would retain similar functionality.

• Use ReduxJS for application state management. Redux is well known among the JS community, and while it is criticized for requiring a lot of boilerplate to get started, I find it to be a super useful library for managing shared application data and state. Additionally, the Redux Dev Tools are enormously helpful for seeing what’s going on during state changes when debugging.

• Use Webpack as a module bundler along with the Babel, Sass, etc. Previously I was using GulpJS as a build system, but the Gulp file I had was messy and not taking advantage of modern features such as bundling ES6 modules, transpiling, and tree shaking. I originally began rewriting the Gulp file and upgrading its dependencies, however it quickly started to feel like going down a rabbit hole to get it to do what I wanted. Ironically, with all the criticism Webpack seems to get for being complicated and difficult to configure, I feel that I know it well at this point from having used it in so many other projects, so decided to make the switch.

• When possible, host any 3rd party JavaScript dependencies instead of relying on CDNs for them. Previously dependencies were either linked to via a CDN or copied to a directory in the git repository. Now most 3rd party dependencies will be installed using npm or yarn and kept out of version control.

• Favor native browser APIs such as fetch and remove some existing JavaScript and CSS dependencies like jQuery, CartoDBJS, and Leaflet that aren’t absolutely needed. Cutting down on JS and CSS has the benefit of helping the site load faster, especially on slower or spotty network connectivity.

  • For example with the map on slide four that shows properties that likely have rent stabilized apartments, I decided to use d3-geo and d3-tile with the Carto Maps API to render the map. The map is essentially what amounts to a static image (it doesn’t allow for zooming, panning, click, or hover interactions) so there’s no need for a full featured web mapping library like Leaflet or MapBoxGL. In fact, when I later saw this Observable Notebook on how to make a webmap from scratch, I realized I could even get by without d3-geo and d3-tile! Sigh.

• However, in some cases I decided to keep an existing dependency. For example, I retained the GSAP web animation library for smooth scrolling behavior. Although there’s now browser support for native smooth scrolling, it isn’t 100% supported by Safari and would require a polyfill for IE. While investigating what seemed like the recommended polyfill, it didn’t seem like it would even solve for my use case. Even with the native browser smooth scroll, you can’t control the duration of the transition or its easing property like you can with GSAP’s ScrollToPlugin. So instead of removing GSAP I upgraded it from version 1.x to 3.x. This had the additional benefit of using GSAP and the ScrollToPlugin as ES modules, so I could incorporate them into my Webpack build system and not load them from a CDN as globals like I was previously doing.

• Write unit tests
  • This is a part of what MF advocates for to make the refactoring process go smoother.
  • Having unit tests can help you feel more confident that you’re not breaking anything when making changes to the code and seeing that the tests pass.
  • Writing tests was a new challenge to me, I typically don’t ever write tests at all!
  • It definitely made the entire refactoring process feel slower as I ended up writing 2-3x as much code as I originally anticipated.
  • I learned a lot though; deciding what to test and writing good tests can be fun puzzles to solve.
  • One payoff of this effort is that the next time I write unit tests for a project I’ll be faster at it.
• Use Netlify
  • The preview deploys that you get for free with Netlify are enormously helpful, e.g. for cross browser testing, CI/CD, code reviews, or when presenting changes to clients and team members who are not developers.
  • Netlify automates building and deploying your app to production (in this case to amirentstabilized.com) when pushing to the main git branch.
  • You get hosting, an SSL certificate (so your site is served over https), logging, and other goodies.
  • The free personal account plan is fairly generous, you only need to pay if you go over build minutes or want premium features such as having multiple team members on an account.

The Refactoring Process

The first step in the refactoring processed involved reviewing the existing JS code to understand how it was structured; I needed to know what each module, code block, and function was doing. Long story short, it was a mess! I had abused the ES5 revealing module pattern, name spaced parts of the code unnecessarily deeply, and used very terse variable names at times which made it difficult to understand what any one part of the code was doing and how it might relate to another. I didn’t do a good job modularizing the code either, there were bits of related logic spread throughout various modules which definitely wasn’t a good organizational practice.

Even though I was aiming to only use vanilla JS I still had bits of jQuery in a lot of places. I teased out logic that was worth keeping and could be improved while also deciding what was worth throwing away. I mainly focused on the JS, deciding not to refactor the styles which use Sass, but I did make small changes or improvements to the Sass code when I felt it was necessary or not too big of a lift.

I began the written refactoring by first creating two entry points for Webpack, one for the main page / app, and another for the three info/*.html pages. This enabled code splitting (for both JS and CSS), which helped keep the bundles generated by Webpack smaller. The main (index.html) page is where the actual “app” is, the other pages are just static content, so they don’t need to load all of the same code. A user may have just bookmarked the resources.html page for example, so when it loads we only serve JS and CSS needed for that page.

I then focused on refactoring the translation & Handlebars template loading logic. This is an important part of code to make crystal clear, as it is essentially how the entire site renders on page load and when a user toggles the language of the page.

The process is roughly:

1. Check for a language code in the browser’s localStorage that may have been previously saved. If nothing is found then default to English.
2. Check for the HTML page’s base name (e.g. “index”, “why”, “how”, “resources”).
3. Load the correct Handlebars template file based on the page name.
4. Load the correct locale JSON file based on the page name and language.
5. Use a dynamic import to grab the correct initialization script (for the index or info pages).
6. Render the page using the Handlebars template and locale JSON.
7. Invoke the initialization script to set up any interactivity.

As mentioned earlier, components were created for all interactive elements on the main page that required JavaScript. An example of this are the buttons such as “Begin” and “Next” that trigger smooth scrolling to a slide further down the page.

To tackle the interactive elements, I created a base Component class to handle common patterns, such as adding and removing event listeners, checking for a DOM node, and providing a cleanup method. Usually I tend to favor the functional programming paradigm when writing JavaScript, but during the refactor I found ES6 Classes to be quite helpful for writing components. For example, I ended up using getters and setters in ways that helped remind me of whether or not a class property could be overridden, and if so, how.

Here’s what that class looks like:

/* Class that all Components which rely on a DOM node inherit from */
export class Component {
  /**
   * @param props.element The Component's DOM node. (Required)
   * @param props.store The Redux store singleton (Optional)
   */
  constructor(props = {}) {
    if ("element" in props && props.element instanceof HTMLElement) {
      this._element = props.element;
    } else {
      throw new Error("Component requires a valid DOM element prop");
    }

    if (
      "store" in props &&
      typeof props.store === "object" &&
      typeof props.store.dispatch === "function" &&
      typeof props.store.getState === "function" &&
      typeof props.store.subscribe === "function"
    ) {
      this._store = props.store;
    }

    this.init = this.init.bind(this);
    this.bindEvents = this.bindEvents.bind(this);
    this.removeEvents = this.removeEvents.bind(this);
    this.unsubscribe = this.unsubscribe.bind(this);
    this.checkForStore = this.checkForStore.bind(this);
    this.cleanUp = this.cleanUp.bind(this);

    this.init(props);
  }

  // Do any other setup work here such as add more class properties,
  // fetch data, etc. Make sure to call this.bindEvents() here.
  // @param Props:  props are passed as an arg from the constructor.
  init(/* props */) {}

  // Add any DOM event listeners
  bindEvents() {}

  // Remove any DOM event listeners
  removeEvents() {}

  // placeholder to unsubscribe from redux store, if subscribed to
  unsubscribe() {}

  // can be used to make sure a store is passed when a component relies on it
  checkForStore() {
    if (!this.store) {
      throw new Error("Requires redux store as a prop");
    }
  }

  // call prior to removing component instance
  cleanUp() {
    this.unsubscribe();
  }

  get element() {
    return this._element;
  }

  set element(el) {
    this._element = el;
  }

  get store() {
    return this._store;
  }
}

And here is an example of a component that inherits from that base class:

import { Component } from "./_componentBase";
import { goToNextSlide, goToSlideIdx } from "../action_creators";

export class AdvanceSlides extends Component {
  constructor(props) {
    super(props);
  }

  init(props) {
    super.checkForStore();

    if ("advanceToIdx" in props && typeof props.advanceToIdx === "number") {
      this.advanceToIdx = props.advanceToIdx;
    }

    if ("buttonSelector" in props && typeof props.buttonSelector === "string") {
      this.button = this.element.querySelector(props.buttonSelector);
    } else {
      throw new Error("Requires a CSS selector for its button");
    }

    this.handleClick = this.handleClick.bind(this);
    this.advanceToSlide = this.advanceToSlide.bind(this);
    this.bindEvents = this.bindEvents.bind(this);
    this.bindEvents();
  }

  bindEvents() {
    this.button.addEventListener("click", this.handleClick);
  }

  removeEvents() {
    this.button.removeEventListener("click", this.handleClick);
  }

  handleClick(event) {
    event.preventDefault();
    this.advanceToSlide();
  }

  advanceToSlide() {
    if (this.advanceToIdx !== undefined) {
      this.store.dispatch(goToSlideIdx(this.advanceToIdx));
    } else {
      this.store.dispatch(goToNextSlide());
    }
  }
}

I ended up being quite pleased with writing components this way. I found it to be simple yet flexible enough to obfuscate the need of a JavaScript framework for AIRS. It felt good to know that you can still make highly interactive and dynamic websites in 2020 without using a JavaScript framework.

When appropriate, I extracted a Component’s corresponding HTML from the app’s Handlebars template into a separate Handlebars partial and named it similar to its component class name. This helped make it clear what HTML belongs to what component and could prove to be useful in the future if I eventually do decide to port the code to a JavaScript framework or use a Web Component library.

For each component class I wrote a series of tests. Martin Fowler advocates for tests to make your code more resilient when refactoring it. One recommendation MF advocates for when writing tests that I found to be fun is to deliberately break the tests to make sure they work as expected. Or to see what happens when you deliberately use your code in unexpected ways, and to test for that. Writing tests can be a bit like solving puzzles; it’s not always apparent what to test for and how to test it. Getting to know Jest and friends (e.g. JS DOM) was also a bit of a learning curve. In the end I’m glad I put in the effort, as I know when I write more tests in the future that it won’t feel so foreign or difficult.

Here’s an example test for the above AdvanceSlides component that checks that the component’s click handler is called when the “button” UI element is clicked on. (I know, this should be a real <button> not an <h3>, a pertinent example of why more folks should be taught about accessibility when they are learning web development!)

test("The component's button handles a click event", () => {
  document.querySelector(".go-next.bottom-arrow > h3").click();
  expect(spyButton).toHaveBeenCalled();
});

I mentioned earlier that I chose to use ReduxJS for managing the app’s state. To be honest, I have not used Redux outside of a ReactJS project before, and this proved to be interesting! Typically when using Redux with React you would use the react-redux library’s Provider context and connect helper function to make a Component aware of Redux. Being that I’m not using React, I utilized an observeStore function, which is how components I wrote respond to changes in state:

// code credit: https://github.com/reduxjs/redux/issues/303#issuecomment-125184409
export function observeStore(store, select = (state) => state, onChange) {
  let currentState;

  function handleChange() {
    let nextState = select(store.getState());
    if (nextState !== currentState) {
      currentState = nextState;
      onChange(currentState);
    }
  }

  let unsubscribe = store.subscribe(handleChange);
  handleChange();
  return unsubscribe;
}

It takes three arguments: the Redux store singleton, a function that returns the piece of state that should be watched for changes, and a callback function (onChange). It returns an unsubscribe function, and recall that the base component class I created has an unsubscribe method. When a component uses this observeStore function, that method is overridden with the function returned by observeStore. This is important, because when the app re-renders as a result of a language toggle, the entire DOM is essentially blown away and any previous component instances need to be cleaned up. That clean up work involves unsubscribing from the Redux store.

For the remainder of the refactoring effort I added Redux boilerplate as needed, e.g. the action types, action creators, reducers, middleware, etc. Redux accomplished somewhat simple tasks such tracking the active slide’s index, as well as more complex and asynchronous tasks such as fetching data from an address geocoding API (Shout out to NYC Planning Labs for the terrific API!). Other “slices” of state include whether or not someone’s address is likely to have rent stabilized apartments, and whether or not the searched address is within a catchment area of a local tenants rights group. These types of data are shared between components and thus benefit from being stored in shared application state.

I particularly like using the Redux Dev Tools for debugging the Redux state, which I find to be a much better UX then console.log‘ing things. The Redux Dev Tools are something you would also have to create yourself if using a custom made pub/sub routine, another solution I considered for managing application state but ultimately decided against using. Lastly, Redux is fairly simple to write tests for, with the exception of asynchronous action creators which can be a little more tricky to test.

That sums up the majority of the refactoring work. Other bits included writing utility helper functions and constants (other than the Redux action types) that get shared between multiple modules. Please feel free to take a look at the app’s source code to learn more or tell me what you think!

Outcomes

The refactor of AIRS ended up being over four hundred commits over a period of three months! Doing this in my free time was not easy, but I found that I could chip away at it here and there to slowly make progress. There were definitely some big pushes at times and at some point I had to decide when to call it “good enough” and merge the changes. I didn’t get around to everything that I had wanted to do in the refactor, however I am now able to tackle work incrementally more easily than I was able to before. Finally, I decided on adding a Changelog file to track significant changes to the code and design of the website. I should probably add a Code of Conduct as well for contributors, as well as a License file.

Oh the things you learn as time goes on! Looking back at my original code helped me reflect on how much I’ve grown as a programmer and web developer, how much the world of browsers and web development has changed, and how my outlook on building websites and UIs has evolved. One example is that the concept of “componentizing” the UI was just beginning to take off around 2013 to 2015, and now in 2020 it’s so entrenched in how us developers build UIs it almost seems inconceivable to do otherwise. Browsers have of course changed in the past five years as well; new APIs are being unveiled while more features become standardized across browsers (well, sort of). Webpack was still fairly new in 2015 and not widely used as a frontend build tool; Grunt and Gulp were still the popular choices back then. While I’ve always valued User Experience above all, I’ve come to appreciate it even more from my current job as a UX Engineer at Google. This has motivated me to fix the accessibility issues with AIRS; for example improper uses of headings or <div> elements that function like as buttons but without the proper ARIA attributes and keyboard event handling.

Metrics:

Here are some bits of quantitative information related to the code before and after the refactor.

Total Amount of JavaScript

• Before:
  • 1MB total JS
  • 48 kB for the bundle.js*
• After:
  • 416 kB total JS
  • 206 kB for the vendors.js bundle
  • ~50 kB total for multiple source bundles

*In the original code some 3rd party dependencies were included in the bundle while others, such as jQuery, were loaded over CDNs.

Following the refactor 584 kB (over half a megabyte) of JavaScript was eliminated!

In both cases the majority of the JavaScript comes from 3rd party dependencies. Following the refactor almost all 3rd party dependencies moved to a vendors.js bundle that can be cached by the browser. This is beneficial, for typically the source code changes more frequently than 3rd party dependencies, and with cache busting file names thanks to Webpack, source code bundles will be received by the browser when they are updated. A few remaining scripts are still loaded via CDNs: the “Add To Calendar” widget, Google Analytics, and the “Add This” social media widget. While this is not ideal, I’m willing to live with it, and may end up using alternatives to some of these in the future anyway.

Amount of Source Code

The amounts listed below are total lines of code:

• Before:
  • JS: 1,880
  • SCSS / Sass: 3,240
  • Handlebars templates: 848
• After:
  • JS, total: 5,839
  • JS, excluding unit tests: 2,122
  • SCSS / Sass: 3,115
  • Handlebars templates: 867

Although the amount of JavaScript increased by close to 4k lines of code, the majority of that consists of unit tests. When excluding the unit tests, the increase was only 242 lines. This is a point MF brings up in his book Refactoring; a refactor may result in more code than existed previously. However this is not necessarily a bad thing! If a refactoring effort results in code that is better structured and more understandable, that is a boon for humans who work on the code, despite having more lines of code to read over. On the surface this philosophy ignores optimization, however MF argues that the priority of writing software should first be to make the code well structured and understandable. After this goal has been achieved performance bottlenecks should then be identified and fixed. To me this is a more reasonable approach than trying to optimize code prematurely. I’d much rather work on code that is well structured and easy to reason with then work on code that is terse, inconsistently organized, and/or difficult to understand.

Lighthouse Score

Results from the Lighthouse auditing tool in the Google Chrome browser.

• Before:
  • 96 Performance
  • 72 Accessibility
  • 71 Best Practices
  • 80 SEO
  • 0.7s FCP
  • 0.9s TTI
  • 0s TBT
  • 1.3s LCP
• After:
  • 96 Performance
  • 81 Accessibility
  • 79 Best Practices
  • 80 SEO
  • 0.7s FCP
  • 0.9s TTI
  • 0s TBT
  • 1.3s LCP

(FCP: First Contentful Paint, TTI: Time to Interactive, TBT: Total Blocking Time, LCP: Largest Contentful Paint)

It shouldn’t be surprising that the results from before and after the refactor do not differ by that much, as I did not focus on making changes that would drastically affect things such as SEO, accessibility, Time to Interative, etc. I did not aim to improve performance other than reducing the amount of JS sent over the wire, which seems to have not affected the performance score. It’s worth noting that the Lighthouse scores are estimates, so re-running the tool may give slightly different results each time. The Lighthouse audits were run on my Mac Mini, circa 2018 with 32 GB of RAM. The scores tended to differ drastically from one run to the next when I ran them on an older laptop.

Possible Next Steps

Here is a short list of tasks that I would like to tackle next, now that the refactor is complete:

• Add integration or end to end tests
• Fix accessibility issues
• Refactor the SCSS
• Integrate a backend service to remove the need for a CARTO account
• Instead of Handlebars, use plain old HTML and a locale JS library such as i18next

Phew, that was a lot! If you made it this far, then thanks for reading. Hopefully this post will motivate you to do some refactoring of your own if it’s something you haven’t tried out yet. And lastly, don’t forget to read Martin Fowler’s book Refactoring!

Happy refactoring!