Friday, 13 September, 2019 UTC
Summary
Legacy code is a fact of life. Here at Benchling, we’ve been writing scientific tools since 2012, and technologies, best practices, and styles have shifted over time. We’ve been happy to adopt technologies like React, Redux, and TypeScript, but what do we do about the code we wrote before introducing those? We have hundreds of thousands of lines of code that we’d love to keep modern and pristine at all times, but it isn’t always good business!
Every modernization effort we’ve taken on has been its own unique engineering challenge and has been influenced by many real-world parameters. What value do we get out of migrating? How much time can we reasonably allocate? How much risk can we accept? Can it be automated, and how likely is automation to actually save time or reduce mistakes? How time-sensitive is it? In the past, I wrote about converting CoffeeScript to JavaScript at scale by stabilizing and running a large-scale migration tool, but certainly that approach doesn’t work for everything.
In this post, I’ll go over a specific case study: porting 125 Jade templates (3000 lines total) to TypeScript React components. We explored the space of technical options for the migration, and ended up with a process that we were happy with: run side-by-side implementations to let production user actions safely exercise the many edge cases. The approach let us perform a large migration with minimal engineering effort and without introducing any bugs.
Quick technical note: in 2015, the Jade project was renamed to Pug for legal reasons. We never ended up upgrading since then, so our files since then have been .jade and compiled using the jade package. I’ll use the word “Jade” throughout this blog post since we’ve still referred to it as that internally, but the modern name for the tool is Pug.
Why it matters
Imagine you’re an excited, motivated engineer who just joined Benchling, a startup that works primarily in React. You’ve gotten comfortable with JSX, the mindset of immutable state and pure functions, presentational vs. container components, and company-specific style decisions and conventions. Then you’re given a task to update an icon in an older part of the UI. You look up the code for it and stumble upon this Jade file:
https://medium.com/media/5b51c5be279e98a07f5a43c467457a94/hrefThat’s not React at all. Then you track down the JavaScript that renders the Jade template, and it’s modifying and building DOM nodes with jQuery code like $('<li>').append($anchor) and $chooseButton.parent().removeClass('disabled'). It’s not wrong, and it may not even be bad code, but it’s jarring and unfamiliar.
You update the icon, only to discover that the template is broken now! Like many editors, yours removes all trailing whitespace on save, and the two trailing spaces at the end of line 5 were necessary. (I bet you didn’t notice those, but they’re there!) Zero trailing spaces causes the template to crash and one trailing space causes a visual bug, so we need both trailing spaces. If trailing spaces matter in Jade, who knows what else you could be overlooking? What does that = mean, and how does it compare with |? On the surface, it’s a clean shorthand for HTML and some conditionals, but you have no idea what details are really lurking. You keep a note to be more careful and work more slowly with this sort of code next time.
Working in unfamiliar technology is scary, and scared programmers are rarely productive programmers. New technologies often have real benefits over old ones, but having many different technologies in the same codebase makes it harder to learn and harder to work with. That’s why my preference is to focus on the legacy technologies that are easiest to completely remove. In late 2018, we brainstormed modernization projects, and removing Jade stood out as a great candidate for that reason.
Jade at Benchling
In the early days of Benchling, all UI was written using Backbone for the state and view logic and Jade for the templating. This coding style was prevalent across the DNA tools that have remained one of the core components within Benchling. For example, most of the UI shown here was powered by Jade and Backbone:
In late 2014, the engineering team decided to bet on the up-and-coming React library, and to start using it for the new Lab Notebook project and all future projects. There were about 5 engineers on the team at the time. As the team grew to 30 engineers, the scope of projects grew as well, and we started using React, Redux, and now TypeScript for new projects.
Now we’re returning focus to improve the foundational DNA tools that started it all. Over the years, these tools have remained a central part of the platform, and their usage has grown significantly. To keep up with the fast pace of science, we need to be comfortable maintaining this code.
Having so much widely-used legacy code puts us in a tricky spot: we want to modernize it, but even small bugs introduced during the process need to be taken seriously. There’s some test coverage, but not enough to give us good confidence that the visuals and interactions are all working right. Even verifying manually is tricky because of the wide feature breadth and science-heavy domain. We can’t just go wild and smash out some React code—we need to be careful and conscious in our approach.
Jade vs. React
So what does a Jade to React port really entail? Let’s look at a tiny example, the “active enzyme” component, which is one row of this table:
Each row displays the name of a restriction enzyme with an auto-assigned color, and lets the user click to remove it.
Here’s the existing code:
https://medium.com/media/eeb6dc7fee35b9f9092faee452934413/hrefhttps://medium.com/media/eb8dfc7afe40f38a387e0d78aa2a9887/hrefHere’s roughly what we want it to look like:
https://medium.com/media/a1c0bf75709ffdebddae35d8deb0b903/hrefConverting this one component isn’t so bad, but this is just 3 lines of jade out of 3000 total, and some Jade templates are hundreds of lines long. For the project to be successful, we’ll need a way to make it faster and safer than porting by hand.
Thinking through the approaches
Let’s brainstorm the approaches that we can take, some of which could possibly be used in combination.
Idea: write tests so we can refactor with confidence
When modernizing legacy code, probably the first thought should be to make sure it’s well-tested. If we had as much time as we wanted, writing tests for all 125 components would allow us to refactor and rewrite the code without worry. In this case, however, there’s a bit of a chicken and egg problem: a downside of Jade is that it’s harder to test. We have lots of tools, utilities, and institutional knowledge on testing React code, but very little precedent for Backbone and Jade. Writing tests is also a serious investment, and for such UI-heavy code it’s hard to be confident that we’ve kept the look and feel even with heavy test coverage. In the end, we want to find a way to port the code over safely without extensive test coverage.
Idea: keep side-by-side implementations
One style of migration is to create a shim: we run the old and new implementations in parallel and make sure they agree. There’s a family of Scientist libraries for implementing this sort of shim, but the concept is simple enough that it doesn’t necessarily need a library: just run both implementations, compare the results, log any differences, and return the result of the existing (trusted) implementation. The trick is figuring out when it’s applicable at all.
Could that work here? After some thinking, it seemed challenging because of the differences between Jade/Backbone and React:
Could that work here? After some thinking, it seemed challenging because of the differences between Jade/Backbone and React:
- Jade/Backbone works by generating a big HTML string for the initial render, then in response to events, you use jQuery to access and modify the DOM.
- React doesn’t use HTML, it uses an object structure corresponding to HTML, then mounts and manages the DOM nodes and uses virtual DOM diffing to change the UI in each subsequent render.
Maybe we could keep a fake DOM in the background and make sure it agrees? Maybe we could do an initial render, verify the DOM, then delete the node, or compare the JSX object structure with the HTML that Jade generates? A fantastic solution didn’t immediately come to mind.
Idea: run the Jade compiler
The jade compiler converts .jade files to .js, so maybe it would give human-readable output that we could use as a good starting point? Let’s see what the generated JavaScript is for activeenzyme.jade above:
https://medium.com/media/54639dd2f8665b960fab748f39526010/hrefNope, not even close. We could certainly run this conversion to technically get rid of Jade, but maintaining it would be much harder than maintaining Jade.
Idea: use an automated migration tool
We moved from CoffeeScript to JavaScript using decaffeinate, so maybe something like that exists for Jade to React? Sadly, an internet search only found one or two projects, none of which were close to mature enough for our use case.
Idea: build an automated migration tool
If a migration tool doesn’t exist, maybe we can make one? This also seemed like a bad idea. When you have hundreds of thousands of lines of code, an automated converter starts becoming worth it. With only thousands, some tedious manual work will likely do better. We can still automate what we can, but a productionized large-scale converter would be a bigger project than we want to take on.
Coming to a decision
Of all of these ideas, the side-by-side approach felt the most promising, but the details require some thought. In thinking through the possibilities, I came to a key realization: React has a server-side rendering mode that takes props and returns HTML, which is exactly how Jade behaves. As it turns out, you can run this mode in the browser as well! A client app that uses React server-side rendering certainly isn’t the same as a real React app, but it gets us a lot closer.
It was a promising idea, but certainly wasn’t guaranteed to work. For example, even if I can write two UIs that look the same, React and Jade might produce HTML that can’t reasonably be compared. I decided to try it and see.
The migration
Here’s the first version of the function we used to compare the Jade and React results:
https://medium.com/media/a61f53ef48f7f5ae6b1afbe7e398b0bd/hrefTo make this safe, it needed a few more details:
- assert (which crashes on failure) is too aggressive, but just logging an error isn’t great either. In development and tests, we want mismatches to be as loud and prominent as possible so that we don’t miss them and so that we can iterate quickly. To simultaneously keep the migration safe and allow for fast iteration, we made the assertion code detect if it’s running in production or development, and log an error (without failing) in prod and throw an exception in dev.
- What if the React rendering crashes? Since the React side is an experiment, we really want to ignore (and report) all errors, so I wrapped that part in a try/catch.
- What if the HTML is only semantically identical rather than exactly identical? I ended up writing a function assertResultsEqual that does a loose comparison; see further down for details.
With these safeguards in place, there’s very little that can break, so writing and reviewing code can happen much faster. Rather than being slow and careful, I can get a quick implementation of each component out there and let the production validation tell me if I made any mistakes, which ultimately is what made the migration so successful.
I tried a few examples and came up with a step-by-step process that I was happy with:
- Pick a good set of files to convert. The typical cadence was about 10 Jade files per code review, but really it was best to pick a group of files that were in the same directory or could be exercised together.
- Generate stub components. I wrote a Python script that takes an array of Jade template paths, creates TypeScript React component files in a standard place with standard naming conventions calling makeJadeTransitionShim, and updates all imports to import the shim rather than the .jade file. The React component just returned <span>TODO</span>.
- Exercise the code, paste initial HTML into the component. With the shim in place, the UI is expected to crash with an error that includes the expected HTML. An easy starting point is to copy and paste this into the render function. PyCharm/WebStorm has a nice feature where you can paste HTML inside JSX and it’ll auto-convert the small differences like class vs. className. This also forced me to learn what area of the product renders this component; the product is complex enough that it wasn’t always obvious!
- Fill in any logic and parameters and figure out any remaining details. This is the part that requires the most thought. Usually Jade conditions and loops map pretty cleanly to JSX code, but not always. This was also an opportunity to fill in TypeScript types for each component.
- Send for code review and deploy to production. Since the new code is all guarded behind the shim and returns the Jade result, the code reviewer doesn’t need to carefully go through each line of JSX code. This made code reviews go much faster.
- Watch the logs and fix any issue that comes up. Our error logging shows the expected and actual HTML, so when there were errors, I put the two through a diff viewer to see where the problem was and figured out how to exercise the problem locally, then fixed it.
Rather than splitting the work up among the team, I decided it was most efficient for one person (me) to do all of the conversion work. We want newer folks to never need to learn Jade, so it wouldn’t make as much sense for them to help out.
A concrete example
Here’s how I ported the activeenzyme.jade file from above. As you’ll see, it feels a lot like test-driven development.
As a reminder, here’s the Jade code we’re porting:
https://medium.com/media/eeb6dc7fee35b9f9092faee452934413/hrefFirst, I ran a script to generate a stub component with the right naming conventions:
https://medium.com/media/17c5cc851444c9908a15425b1f203e38/hrefThen I opened the Enzymes panel in Benchling and immediately got this error (differences bolded):
Assertion Error: Expected Jade and React results for ActiveEnzyme to match.
normalizedJade: <td class="mono">Acc65I</td><td><div style="background-color: #F58A5E;" class="swatch"></div></td>
normalizedReact: <span>TODO</span>
The HTML here is two elements, so in order to paste into the component, I wrapped it in a fragment using the <> syntax:
https://medium.com/media/1308abd975b29f705f765e132eae17f6/hrefAfter refreshing and trying again, React gave this error:
Error: The `style` prop expects a mapping from style properties to values, not a string.
Makes sense, JSX and HTML aren’t exactly the same. I changed it to <div style={{backgroundColor: '#F58A5E'}} className="swatch" /> and it then gave this error:
Assertion Error: Expected Jade and React results for ActiveEnzyme to match.
normalizedJade: <td class="mono">Acc65I</td><td><div style="background-color: #F58A5E;" class="swatch"></div></td>
normalizedReact: <td class="mono">Acc65I</td><td><div style="background-color:#F58A5E" class="swatch"></div></td>
Hmm, this one is more interesting. The Jade code has an extra space and semicolon, but in this case that’s added explicitly, so an easy fix is to tweak the Jade code so the two can be compared. I changed div.swatch(style='background-color: ' + color + ';') to div.swatch(style='background-color:' + color) in the Jade code and re-ran it and got this error:
Assertion Error: Expected Jade and React results for ActiveEnzyme to match.
normalizedJade: <td class="mono">AccI</td><td><div style="background-color:#B1FF67" class="swatch"></div></td>
normalizedReact: <td class="mono">Acc65I</td><td><div style="background-color:#F58A5E" class="swatch"></div></td>
Making progress! Hard-coding Acc65I and #F58A5E was the easy first pass, but obviously we should be showing the real enzyme name and color based on the parameters:
https://medium.com/media/8b2169f641e78da12c464465fd4e2d41/hrefI tried it and it worked! I put this up for code review, and the only thing for the reviewer to confirm was that I was using the framework correctly and that the Jade change was ok. With the review accepted, I shipped the code to production.
Over the next few days, some warnings started to show up in the logs:
Over the next few days, some warnings started to show up in the logs:
Assertion Error: Expected Jade and React results for ActiveEnzyme to match.
normalizedJade: <td class="mono">ApoI</td><td><div style="background-color:null" class="swatch"></div></td>
normalizedReact: <td class="mono">ApoI</td><td><div class="swatch"></div></td>
It turns out that a bug made it so color is sometimes null, and the Jade code was using an invalid style in that case! Because of the strict HTML validation, we found a bug that otherwise would have gone hidden.
Comparing the HTML
In the ActiveEnzyme case, we were lucky that the HTML strings were exactly identical after some tweaks. Other cases weren’t as easy. For example, when rendering a checkbox as checked, React uses checked="" while Jade uses checked="checked". They both work, but the HTML is no longer identical. To fix these issues, I iterated on a comparison function that canonicalizes both the Jade and the React output to make them === comparable. See the full code in this gist if you’re curious. Certainly a lot of cases to handle, but I was able to add them as I ran into errors, and missing cases were always silent to users.
What went well?
The full migration away from Jade happened over the course of a few months and spanned about 15 PRs that each converted about 10 files. The total engineering time spent was about 5 to 10 days, with lots of time interspersed.
A major success was that the migration was completely safe and caused zero user-facing issues. There were about 20 non-user-facing errors reported to the logs, about half of which would have been user-visible errors if we weren’t using a shim.
In addition to the safe migration, we discovered and fixed about 5 pre-existing bugs. As one example, we had a bizarre issue where unexpected text showed up in the UI in very rare situations. It turned out that a Jade template was using name as a variable without it being passed in, so if a browser extension set window.name to any real value, it would show up in the UI! The React version was more strict and crashed.
What would the process have been like if we had converted each component by hand-writing the JSX? It’s impossible to know for sure, but I’d estimate that it would take at least 3 total days per code change and would have introduced several user-facing bugs in the process. So it seems safe to say that the strategy sped up the migration by at least 3x.
What didn’t go as well?
Maintaining Jade and React files side-by-side led to some confusion frustration within the team. While the code was in a dual validation state, someone made a codebase-wide change to the way that React <button>s are rendered and was surprised to see Jade/React difference errors that weren’t covered by tests. This wouldn’t have caused any user-facing errors, but it was jarring and another thing that everyone had to keep in mind. Ideally, we would have moved off of the intermediate validation state sooner to avoid confusion.
One reason that the original Jade code stayed around for so long was that the original project scope was a bit too ambitious. Rather than trying to fully move to idiomatic React before removing Jade, I decided to work more incrementally: write a simple drop-in replacement for Jade, powered by React, and go with that for now.
Takeaways
Certainly, our code modernization efforts at Benchling aren’t done. We’ve moved off of Jade as a frontend template language, but if we had as much time as we wanted, we’d fully move Python 2 to 3, jQuery promises to ES promises, JavaScript to TypeScript, and Backbone to React, and plenty more technology upgrades, both internal and external.
These challenges vary wildly in their difficulty and their value, and need to be weighed against the many features, bug fixes, polish items, tests, and other code changes we might make. Large-scale code changes can be scary and intimidating at first, but a few tricks from this post can make them more manageable:
- Work incrementally! It’s often ok to put code in a complex intermediate state if there’s a clear path to make it simpler than before and the team is on-board with the changes.
- Try to find some way to run new and old implementations side-by-side and compare the results. This sometimes means reducing the scope of the change, like how the Jade templates were converted to React templates rather than true dynamic React components.
- Remember that your users often exercise your code better than your tests do. If you aren’t sure about an assumption or an alternate implementation, just add production logging. At the same time, don’t let users suffer from your mistakes; failing fast only makes sense when you don’t have a safe fallback, and safe fallbacks are critical here.
- Strive to make large-scale changes as mindless as possible. Code changes requiring care and focus will always be slower and introduce more bugs than code changes that naturally work as long as the tests pass.
Certainly these techniques won’t make all code migrations easy, but they can help lower the barrier to make modern development more attainable, even for the oldest legacy code.
If you’re interested in building tools to push science forward, we’re hiring!
Discuss on Hacker News
Thanks to Aaron, Jeff, Naomi, Ray, Saif, Somak, Stephen, and Vineet for reading drafts of this.
Jade to React: using prod validation to make modernization safe, fast, and fun was originally published in Benchling Engineering on Medium, where people are continuing the conversation by highlighting and responding to this story.