Sloppy mode block-scoped self-defining functions don't hoist well

[littledan]

There's an idiom called "self-defining functions". It usually looks like this:

var foo = function() {
  foo = ...
}

Some users (e.g., GWT, which has widely deployed compilation output all over the web), write a slightly different version, used to implement Java static {} class initialization blocks that run just once, but with checks all over the place to run it if it hasn't been run yet:

function foo() {
   foo = ...
}

Many module systems wrap all the definition code in a try/catch. Google does this internally, and the JS code in Reddit seems to have had a similar treatment. If we have a sloppy-mode block-scoped self-defining function as follows:

try {
  function foo() {
    alert("hey!")
    foo = () => {}
  }
} catch (e) { }
foo()
foo()

then (I believe) all browsers pre ES2015 (IE10-, WebKit, Firefox and Chrome) would alert "hey" just once. This is the long-standing web-compatible intersection, as far as I can tell.

However, ES2015 semantics would say that there are two bindings--the inner lexical one inside the try block, and the outer function-hoisted one. The line redefining foo will redefine the lexical binding, and leave the hoisted one in place. So "hey!" will be alerted twice. This is IE11 and Edge semantics, as the first implementors of something approximating Annex B 3.3.

I don't think these semantics are web-compatible. They break Google Inbox, though Inbox does not currently try to work on IE. More generally, the breakage is just the result of a combination of two techniques that independently work and are independently widely used--self-defining functions (through a rather odd syntax that GWT has been emitting for years), and surrounding function definitions by try/catch (which many module systems seem to do). It is, at the very least, a refactoring hazard to prohibit them together.

I think it's possible that this breaks the web even though IE11 has been shipping for a couple years because

• GWT is used in a lot of deploy-once intranet applications which might be only used on particular browsers, and might hit only old versions of IE or non-IE browsers.
• Lots of static initialization blocks could be run multiple times, and the program can still sorta get by, but that doesn't make its behavior correct.

I think the second binding, the lexical one, isn't necessary to get something working that will allow references to lexically scoped variables from hoisted functions and be within the intersection of all browsers' semantics. An alternative semantics suggested by adamk@ is to have just one var-like binding, initialized to undefined, which is set at the top of the block where the function declaration occurs to that function value. There would be no conditionality semantics, and no multiple bindings for the same variable name, so the result would be easier for both users and implementors to understand.

[allenwb]

Wow, I've never seen that idiom before. I wonder if anybody other than GWT uses it.

I'd suggest starting by making the smallest possible accommodation for the specific idiomatic use that has been observed. In this case what we would look for would be:

1. A try block whose that only contains a FunctionDeclaration
2. The function body contains a LHS reference to the name of the function.

Because the block only contains the declaration, its only possible reason for existing is to introduce a hoisted function declaration. It isn't even possible to call the function if it isn't hoisted so there is actually no need for a block level lexical binding to be created. You probably can even get away without checking for the second condition.

So, I'd look for this specific pattern and hoist without creating a lexical binding.

However, all the other restrictions on when to apply function declaration compat fixes should apply as we don't want this semantics being used in new code. In particular, you should never do such an implicit hoist that crosses an outer lexical declaration (let, class, etc) of the same name.

[littledan]

I think it's fairly widely used. A version of the idiom is described in https://www.safaribooksonline.com/library/view/javascript-patterns/9781449399115/ch04.html under the heading "Self-defining functions", though that uses the first form of the syntax.

I'm a little uncertain about this restriction. It sounds complicated to implement and counter-intuitive for users. Users have always had a single var-style binding for sloppy-mode function declarations across all browsers, in all contexts. We didn't do a "lexical fixup" for var with these hoisting constraints; if you have a var which is declared under a let, it doesn't get an implicit hoist crossing an outer lexical declaration--it just throws. What if we did the same for sloppy-mode function bindings?

I see a few problems with the extremely narrow fix you described:

• With eval, we can't determine in advance whether a function body contains an LHS reference to the name of the function. By the time the code runs in eval, it would be too late. This would be the first time ES contained such an inconsistency with respect to eval.
• The try blocks here contain both FunctionDeclarations and other statements, so the formulation is too restrictive. We would probably want to allow arbitrary try blocks
• The mental model here for programmers is pretty inconsistent. When they make a little change, all the sudden the semantics of the variables are different, due to this or that edge case. Do we really need this complexity? With this proposal, there are three ways that functions in sloppy-mode functions can be bound: Just with a lexical binding, just with a var-style binding, and with both.
• The var-style binding in Annex B 3.3 is defined to be set by reading the lexical binding and writing it to the var binding. Sounds like different machinery will be needed for this new type of hoisting. Also, to allow functions to be used in a top-down way within a try statement, setting the variable will have to be hoisted to the top of the block. This amounts to all the complexity of @ajklein's proposal, except implemented just for this one edge case.
• For implementations, it'll create some amount of overhead in the parser to be constantly testing whether function bodies have LHS references to the same function anywhere inside them lexically. There's not currently spec machinery or implementation machinery for doing this. Delaying creating any binding at all will also be somewhat difficult for implementations like V8, which attempt to do a few things in the first parser pass--we'll have to go back and fix up code that's already been parsed, recursively through all inner scopes.

I hope we can come up with simple, consistent semantics that satisfy all the use cases.

[bterlson]

It is notable that IE11 implemented essentially the current Annex B semantics and these are still implemented in Edge as well. This amounts to millions of users and none of these have reported sites broken due to this issue. It would be great to get a sense for if this pattern is actually used broadly enough to justify either adding a lot of complexity with a targeted fix as @allenwb proposes, or b) eliminating block-scoped function bindings in sloppy mode.

[eplawless]

We use this pattern internally at Netflix. It's not very common, but it appears probably at least once in each of our projects. It's used mostly to lazily allocate some resource the first time a function is called.

function calculate(arg) {
  var resource = createExpensiveResource();
  calculate = function(arg) {
    doSomethingWith(resource, arg);
  };
  return calculate(arg);
}

[allenwb]

@eplawless But do you ever use such declarations inside a block (such as a catch block)? There is no problem with this pattern if at occurs at the global script level or the top level of a function.

[eplawless]

@allenwb I'd have to search for it; offhand I don't know if we do. I wouldn't be surprised to see it appear within an IIFE, though. Additionally, I know of at least one user-facing project whose build process is just to concatenate the source files and wrap the whole thing in a try/catch.

I think the pattern is reasonably idiomatic, and I would be very surprised if its behavior changed in the way described.

[allenwb]

@littledan

Users have always had a single var-style binding for sloppy-mode function declarations across all browsers, in all contexts.

That is not correct. Haven't you look at the previous analysis that was done by @bterlson, etc. There are only a limited number of legacy usage patterns for block level function declarations that are actuallyt inter-operable across all browsers. Apparently, you have identified another one.

We didn't do a "lexical fixup" for var with these hoisting constraints; if you have a var which is declared under a let, it doesn't get an implicit hoist crossing an outer lexical declaration--it just throws.

Because let declarations didn't exist on the web until ES6, so there can't be any pre-existing code that does this that we need to preserve.

What if we did the same for sloppy-mode function bindings?
we do, it is never legal to hoist a function declaration over a like-named let declaration

Edge has already demonstrated that the solution given in B.3.3 is implementable. You new case is actually simpler than the one that B.3.3 already deals with as it doesn't require two binding, just the hosted one. Essentially, for such function you would revert to what ever you have already done.

[allenwb]

@eplawless appearing in a IIFE wouldn't be a problem, because declarations are never hoisted across function boundaries. It's only { } blocks containing functions that are problematic.

[eplawless]

@allenwb yep, makes sense. It's the try/catch wrapping that worries me, which I can confirm we do on one of the libraries which we deploy to a majority of our devices.

[bterlson]

@eplawless is it possible to find where this pattern is used and give me a URL (I have a Netflix account if needed :)). If you can find it, the link won't work in Edge or IE11 which seems very bad...

[eplawless]

@bterlson sure, let me go speak to the team whose library I'm thinking of. It's served to game consoles, media boxes, and smart TVs, so I should be able to find a CDN link for you (hopefully unminified).

[bterlson]

@eplawless that would be amazing!

[eplawless]

@bterlson so, updates:

The team who used to concat their build and wrap in a try/catch has switched to Browserify and does not wrap in a try/catch anymore. In that rewrite they removed the instance of this pattern I was aware of.

Another team does have this pattern but each usage I see happens to be within an IIFE. I want to stress that this is accidental and that at any point a refactor could remove it. I have a minified link for the code that contains this pattern; this is what runs on devices: https://secure.netflix.com/us/tvui/release-2015_11_02-49/13_3/sapphire/720p/gibbon/darwin_main.js

Here's an unminified excerpt:

function shouldLogVerboseRequests() {
    var result = false;
    var showVerboseConsolidatedLogs = util.CurrentUrlParams.get('showVerboseConsolidatedLogs') || '',
        arrToLog = showVerboseConsolidatedLogs.split(','),
        result = arrToLog.indexOf('request') > -1 ||
            arrToLog.indexOf('requests') > -1 ||
            showVerboseConsolidatedLogs === 'true';

    shouldLogVerboseRequests = function memoizedShouldLogVerboseRequests() { return result; }
    return result;
}

I can find 5-10 instances of it offhand, though I haven't been in this codebase for about a year. The behavior change wouldn't make this code incorrect, but it would degrade performance on low end devices (of which we support many) given the high volume of calls to this and similar functions.

That's probably all I can add to the conversation. The other engineers I spoke with were alarmed at the change; I think it is a very surprising behavior.

Edit: fixed URL

[allenwb]

@eplawless

That's probably all I can add to the conversation. The other engineers I spoke with were alarmed at the change; I think it is a very surprising behavior.

Exactly which change is alarming? The ES6 semantics for the above code at the global level of either a script or function has not changed in any way.

It's only if the code was something like:

if (somePredicate()) {
   function shouldLegVerboseResults() { ... }
 }

would anything be different. The above code is already non-standard and not very interoperable. Have you ever seen that combination?

It would also break for:

try {
   function shouldLegVerboseResults() { ... }
} catch (e) {};

that is the case that GWT apparently is producing. But, I think that pattern is specific enough that we should be able to account for it.

[eplawless]

@allenwb

My knowledge of the spec is very limited, but I'm surprised that you mention this pattern is non-standard. Was the fact that a function statement creates a single binding in its containing scope specified for ES5, or is this undefined (or against-spec) behavior that just happens to work?

I think our alarm comes from the fact that a seemingly orthogonal detail (whether the statement is within a block) can impact the behavior of this pattern. I'd expect to be able to reason about this by saying one of "function statements only create an outer binding" or "function statements have separate inner and outer bindings", but I'm not sure I understand why this should behave differently inside a block.