Tuesday, October 15, 2024

Next.js, SWC, and citeproc-js

Last year I got a bug report that Citation.js was not working when built in a Next.js production environment for unclear reasons. Next.js is a popular server framework to make web applications with React, and by default transforms all JavaScript files and their dependencies into “chunks” to improve page load times. In production environments, Next.js uses the Rust-based “Speedy Web Compiler” SWC to optimize and minify JavaScript code.

I was able to figure out that somewhere, this process transformed an already difficult-to-grok function (makeRegExp) in the citeproc dependency into actually broken code. After some trial and error I found the following MCVE (Minimal Complete Verifiable Example):

function foo (bar) {
    var bar = bar.slice()
    return bar
}

foo(["bar"])

// equivalent to

function foo (bar) {
    var bar // a no-op in this case, apparently
    bar = bar.slice()
    return bar
}

foo(["bar"])

But then, in the chunks generated by Next.js, the argument bar gets optimized away from foo(), generating the following code (it also inlines the function).

var bar;
bar = bar.slice();

Now, this is a simple mistake to make. If you expect var bar to actually re-declare the bar argument, the argument is clearly unused and can be removed. Due to the quirks of JavaScript that is not the case though, and the incorrect assumption leads to incorrect code.

This is not a one-off thing though: last August I got another, similar bug report with the same cause: some slightly non-idiomatic code (CSL.parseXml) from citeproc got mis-compiled by SWC. I found another MCVE:

function foo (arg) {
    const a = { b: [] }
    const c = [a.b]
    c[0].push(arg)
    return a.b[0]
}

The compiler misses that c[0] refers to the same object as a.b and thinks that makes the function a no-op, though it does not optimize it away fully, instead producing the following:

function (n) {
    return [[]][0].push(n), [][0]
}

This was apparently already noticed and fixed last May though the SWC patch still has to land in a stable version of Next.js. Interestingly, the patch includes a test fixture that uses CSL.parseXml as example code; apparently citeproc is a good stress-test of JavaScript compilers.

This is all fine with me, I am not going to blame the maintainers of a complex open-source project like SWC for occasional bugs. However, I would like to see a popular framework like Next.js, with 6.5 million downloads per week and corporate backing, to do more testing for such essential parts of their infrastructure. I also do not see them among the sponsors of SWC.

Edited 2024-10-15 at 17:26: Actually, the creator for SWC is also a maintainer for Next.js, though I do not know in which order. Given that, it makes more sense that they switched away from the well-tested but slower BabelJS in version 12, and more confusing why they did not test it a bit more thoroughly.

No comments:

Post a Comment