Contributing to Babel: Three Lessons to Remember

Posted Aug 16, 2017 by Karl Cheng

Getting to work your way around a new code base always poses its challenges, and Babel was no exception.

I’ve been working with Babel as part of the Google Summer of Code 2017 program, working to update Babel transforms and the Babylon parser to accommodate changes to specifications and implementing new features.

Here’s a few things I’ve learnt from my adventures so far.

1. Yes, communication is important

To start off with getting to know the codebase better, I combed through the open issues list on Babel and found a relatively easy one (issue #5728) to deal with.

Just to make sure I knew what I was doing, I fired a quick question on the thread:

After getting clarification, I set off to change the plugin to not throw “runtime” errors during transpilation, but only when the code is actually being run. One incriminating piece of code stuck out:

for (const violation of (binding.constantViolations: Array)) {
  throw violation.buildCodeFrameError(messages.get("readOnly", name));
}

Now what needed to be done here was to actually insert a throw statement into the generated code, which didn’t prove to be too difficult. However, there were still a few cases where runtime errors were being thrown elsewhere from code that wasn’t directly related to this file.

Wanting to go and explore other parts of the Babel code base, I put that down for me to get on with later.

Not too long after, I received a, well, interesting update on the issue… Wait what?

I never actually said I was working on fixing the issue, but assumed that posting would have implied I was going to work on it.

Oops.

2. Where snapshot testing falls short

After setting off for another hunt, I stumbled across issue #5656:

Arguments deoptimized when shadowed in nested function

This is a feature request (I think). Arguments are not optimized if an inner function shadows the name with a parameter (or rest parameters in my case).

Input code

const log = (...args) => console.log(...args);

function test_opt(...args) {
  log(...args);
}

function test_deopt(...args) {
  const fn = (...args) => log(...args);
  fn(...args);
}

…

Expected vs. Current Behavior

I’d expect the code to be optimizable to use .apply( thisArg, arguments ) throughout. However, in test_deopt the outer …args gets copied just to be passed into the inner fn. I can verify that the problem disappears if I rename either the …args of test_deopt or of the fn arrow function.

What’s going on here?

Now what was happening was that this code would generate the following:

var log = function log() {
  var _console;

  return (_console = console).log.apply(_console, arguments);
};

function test_opt() {
  log.apply(undefined, arguments);
}

function test_deopt() {
  for (var _len = arguments.length, args = Array(_len), _key = 0; _key < _len; _key++) { // unnecessary loop
    args[_key] = arguments[_key];
  }

  var fn = function fn() {
    return log.apply(undefined, arguments);
  };
  fn.apply(undefined, args);
}

See that for section there? Usually this is needed as the arguments object isn’t a real array — for example, if you tried to run arguments.slice(), it would fail miserably. However, in this case it’s only being passed to Function.prototype.apply. Surprisingly enough, Babel already bothers to optimize this specific case, like in the test_opt example above.

Trying to fix it

So what did I do? Adding the problem file as a new test case, I tried to see if I could get the output to reflect what I wanted.

“Why’s the test failing? Surely if I change it a little it will solve itself.”

Despite spamming make test-only and modifying the transforms of referenced identifiers within the code, any change just resulted in a different bunch of tests failing instead.

The Chromium debugger is “fun”

Miserable, annoyed and confused, I bothered to fire up the Node.js inspector to step through what was going on.

After returning to my computer from a drink break, I’m gladly greeted to my hard disk light thrashing around and a practically hung computer.

Holding my computer together with judicious applications of Alt + SysRq + F, I managed to work through the flow of things¹ and figure out how exactly the code worked.

Even through all that, I still couldn’t see any reason why it was deciding to remove this “necessary” (so I thought) code that was being removed with my original fix.

The actual problem?

See the error shown above? That entire code in green wasn’t meant to be there, even though it was “expected”.

Basically: the test was broken. Great. :/

The actual fix involved creating a referencesRest function to make sure that the spread operator was actually being applied to the original parameter, rather than a variable in another scope masking the variable.

¹: Turns out that adding a large folder to the DevTools workspace would leak memory until causing an OOM (bug I filed for this).

So why do we use snapshot testing then?!

Well first off, it’s far easier to create tests when all you need to do is ask Babel to run your test case to generate your expected file. This presents to us a low time cost option while protecting against a significant proportion of potential errors.

Also, especially with the type of program Babel is, it would be far harder to test for in other ways. For example, we could check for specific nodes of the AST, but this takes far longer to write and is also prone to non-obvious breakage when your code attempts to change the way the transform is done.

So, all in all, a few lessons here:

1. Make sure your tests are right in the first place—don’t be complacent!
2. Yes, the debugger is actually useful in seeing what goes on.
3. Sometimes things take time to work out—if you’re getting nowhere, take a break or work on something else.

3. Team meetings!

I know this kinda stretches the notion of an “issue”, but anyway :)

When you’re working on a project with a bunch of other people, it’s always useful to catch up with one another and discuss areas which we need to work on.

So how exactly do we go about doing that?!

Ugh, meetings.

When you have a bunch of people spread across the world, finding ways to communicate is never easy, but regardless we would have to make do with our attempts at this feat.

Time zones

When you’re dealing with a open source project spanning all across the globe, picking an appropriate hour quickly turns a rather involved exercise in bikeshedding.

Even with the vast spread between each of us, it seemed like we could just about manage to finally get something together.

Alas, this was not to last. Eventually, we ended up having to switch between two times every other week to accommodate other users (13:00 and 16:00 UTC), which meant that I was only able to attend once a fortnight.

Despite this, we’ve managed to make significant progress with coordinating fixes to various parts that make up key changes to Babel, including support for TypeScript, changes to the order in which transform plugins run, as well as keeping up to date with changes from TC39.

Where to next?

We’re continuing to polish up Babel 7 for general consumption, with a number of new features coming along with that.

I’m working with a bunch of others to get support for updated Class Fields specification proposal included into Babel so that people can test it out and provide feedback.

Also, while I’m at it, I’d like to thank all of the Babel mentors and contributors for helping me out with peer reviews and providing guidance with proposals, all the way from first contact to today.

Looking to find out more about Babel? Hit up our contributing page and join the Slack community!

More about Karl

Karl Cheng is a GSoC 2017 student hailing from Sydney, Australia. Find out more about him on GitHub (Qantas94Heavy) and Twitter (@Qantas94Heavy)!

Please check out our first post on Summer of Code for more info!

Community Discussion