Real World Func Decl in Block Scope Breakages

Brendan Eich brendan at mozilla.com
Wed Dec 26 15:26:48 PST 2012


Brian Terlson wrote:
> 20 sites, however, will likely be broken by this change in some way. 
> There is also a chance that the tool used to identify breakages has 
> missed some code that will breka.
>
> Below are some examples of code on the web today that will be broken. 
> For each I include a snippet of code that is heavily edited in an 
> attempt to convey the pattern used and the developer intent. I also 
> attempt to identify what functionality will actually be broken.
>

Thanks for the snippets. Some questions interspersed below.

> Most of the breakages occur in non-library code, with two exceptions: 
> qTip 1.0, and thickbox 3.
>
> # http://ninemsn.com.au
>
> RenderModal = function () {
>
> if (x) { // is an array of shortcuts that can be added. Is statically 
> non-empty.
>
> function K() {
>
> // process the array of shortcuts in some way
>
> }
>
> }
>
> K();
>
> };
>

Since x evaluates as truthy no matter what (even an empty array is an 
object, therefore truthy), this is equivalent to

RenderModal = function () {

function K() {

// process the array of shortcuts in some way

}

K();

};


but of course it lies outside of ES6's function-in-block semantics, yet 
still in the intersection of Firefox and IE/IE-emulating-browsers.

> # http://yandex.ru
>
> if (_ycssjs("BaH0Fmmo2Sg24lRmTPrK0B8qpaA")) {
>
> function cp(g, c, d) { /* ...*/ }
>
> function csh_ifgsid(c, b) { /*...*/ } }
>
> // ... thousands of lines of code ...
>

This "then" block presumably closes here, before:

> if (_ycssjs("rO+QIoSf2L0NwDn6vjJjy+27nxI")) {
>
> function news() {
>
> csh_if_gsid();
>
> }
>
> }
>
> if (_ycssjs("YRPF0QVjJmhRiKRu6cvi3YXqYo8")) {
>
> // ... bunch of stuff ...
>
> cp();
>
> }
>

Somehow the "if" conditions are logically connected such that no calls 
to an undefined identifier occur! Blech.

> ## Scenario
>
>
> Unknown
>

Indeed! Cc'ing Chaals in case he can shed light.

> # http://g.espncdn.com/nfl-primetime-payoff/en/module/entry?matchupid=478
>
> if (isIE && isWin) {
>
> // ...
>
> } else {
>
> function JSGetSwfVer(i){
>
> // ...
>
> }
>
> }
>
> function checkFlash(myRev) {
>
> // ...
>
> if (isIE && isWin) {
>
> // ...
>
> } else {
>
> aV = JSGetSwfVer(rV);
>
> }
>
> }
>
> ## Scenario
>
> Can’t find a page which uses this code.
>

Can you say how many of the 20 cases involve actual live code using the 
function-in-pre-ES6-block code?

> # http://www.t-online.de
>
> if (!Adition_Environment) {
>
> var Adition_Environment = (function () {
>
> var _this = {};
>
> // ...
>
> _this.getPrf = function (cuId) {
>
> var prf = "";
>
> try {
>
> prf = Adition_Prfstr(cuId);
>
> } catch (e) { }
>
> return prf;
>
> };
>
> // ...
>
> })();
>
> }
>
> // snip 1k lines
>
> if (typeof Adition_Prfstr == "undefined") {
>
> function Adition_Prfstr(ADITION_CONTENTUNIT_ID) {
>
> // ...
>
> }
>
> }
>
> ## Scenario
>
> Required to display advertisements on the page.
>

Heinous.

> # http://manormystery.com
>
> if (!window._ate) {
>
> window._ate = { /* ... */ };
>
> function addthis_open() { /* ... */ } } else { _ate.inst++; }
>
>

Did you snip the closing brace for the "then" block left open, the one 
for "if (!window._ate)"?

> if (_atc.abf) {
>
> addthis_open(document.getElementById("ab"), "emailab", 
> window.addthis_url || "[URL]", window.addthis_title || "[TITLE]"); }
>
> ## Scenario
>
> Social sharing popups broken at least. This may be a general utility 
> used in a number of places.
>

On the other hand, the snippet seems like a mis-coding of

if (!window._ate) {

window._ate = { /* ... */ };

}

function addthis_open() { /* ... */ } } else { _ate.inst++; }


if (_atc.abf) {

addthis_open(document.getElementById("ab"), "emailab", 
window.addthis_url || "[URL]", window.addthis_title || "[TITLE]"); }

...
}

I'd try to evangelize the owner of this code.

> # http://manhunt.net (NSFW, DO NOT VISIT AT WORK)
>
> /** @todo isFlashInstalled() should probably be gotten by include from 
> js/cmmn/flashDetect.js or upfunc.js. */
>
> if (typeof isFlashInstalled == 'undefined') {
>
> function isFlashInstalled() {
>
> var requiredMajorVersion = 6; // Major version of Flash required
>
> var requiredMinorVersion = 0; // Minor version of Flash required
>
> var requiredRevision = 0; // Minor version of Flash required
>
> var s = new SWFObject();
>
> if (!s) return false;
>
> var version = s.installedVer;
>
> if (!version) return false;
>
> return (version.major >= requiredMajorVersion && version.minor >= 
> requiredMinorVersion && version.rev >= requiredRevision);
>
> }
>
> }
>
> isFlashInstalled();
>

Here is the simplest example of a pre-ES6 Firefox/IE+clones 
intersection: function-in-block with hoisting to top of outer function 
or global scope *and* SpiderMonkey's assignment-like runtime binding 
both work the same, provided the use(s) of the function's name all occur 
after any such assignment (in the SpiderMonkey in Firefox scenario).

There's no control flow relation in general, e.g. dominance. However in 
this case the "if (typeof isFlashInstalled == 'undefined')" head could, 
to a sufficiently smart CFG analyzer, imply that isFlashInstalled() 
post-dominates some kind of "definition" of that name.

Ok, enough so far to say a few things:

1. We have time and (among the bigs involved in ES6) try evangelization. 
Evangelization can be crowd-sourced too, and can be more effective when 
done that way.

2. We have some terrible choices, even if ES6's function-in-block 
semantics require "use strict". That leaves the above mess a de-facto 
standard which has never been specified. Specifying the sloppy-mode 
intersection is going to be ugly and a time-sink.

3. The carrot of function-in-block may be sweet -- we have evidence in 
all the good sites your crawl found that are not the insane 20 -- but 
whether "use strict"; required before all such ES6 function-in-block 
uses would be a stick remains to be investigated. Did you check for "use 
strict" or actual enabled-in-IE10 strict mode among the sites you found 
that declare functions in blocks?

/be




More information about the es-discuss mailing list