MatAtBread / fast-async

605 stars 21 forks source link

Strict mode transformation bug on IOS #30

Closed Morkowski closed 7 years ago

Morkowski commented 7 years ago

Code:

class exampleClass {
    async fun1(){}
    async fun2() {
        const channel = await this.fun1();
        try {return await this.fun1();} catch(e) {
            return false;
        }
    }
}
export default EpgController;

which is trasformed to:

(...)
"use strict";
(...)

var exampleClass = function () {
    function exampleClass() {
        _classCallCheck(this, exampleClass);
    }

    _createClass(exampleClass, [{
        key: "fun1",
        value: function fun1() {
            return new Promise(function ($return, $error) {
                return $return();
            }.bind(this));
        }
    }, {
        key: "fun2",
        value: function fun2() {
            return new Promise(function ($return, $error) {
                var channel;
                return this.fun1().then(function ($await_2) {
                    try {
                        channel = $await_2;

                        function $Try_1_Post() {
                            return $return();
                        }

                        var $Try_1_Catch = function (e) {
                            try {
                                return $return(false);
                            } catch ($boundEx) {
                                return $error($boundEx);
                            }
                        }.bind(this);try {
                            return this.fun1().then($return, $Try_1_Catch);
                        } catch (e) {
                            $Try_1_Catch(e)
                        }
                    } catch ($boundEx) {
                        return $error($boundEx);
                    }
                }.bind(this), $error);
            }.bind(this));
        }
    }]);

    return exampleClass;
}();

On IOS I get error Strict mode does not allow function declarations in a lexically nested statement. connected with code:

try {
        function $Try_1_Post() {
            return $return();
        }
}

Any ideas how to fix it?

matAtWork commented 7 years ago

I have seen this before. I seem to remember the solutions are:

Specifically, if you look at the example below, fast-async doesn't generate incorrectly nested functions - nodent (and so fast-async) understands the issue - but if babel has got to it first, this doesn't work. I seem to recall this gets even worse if you use let or const since these prevent some hoisting operations.

Let me know if any of these work for you!

http://nodent.mailed.me.uk/#%22use%20strict%22%3B%0Aclass%20exampleClass%20%7B%0A%20%20%20%20async%20fun1()%7B%20return%20%22ok%22%20%7D%0A%20%20%20%20async%20fun2()%20%7B%0A%20%20%20%20%20%20%20%20const%20channel%20%3D%20await%20this.fun1()%3B%0A%20%20%20%20%20%20%20%20try%20%7Breturn%20await%20this.fun1()%3B%7D%20catch(e)%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20return%20%22OOPS%22%3B%0A%20%20%20%20%20%20%20%20%7D%0A%20%20%20%20%7D%0A%7D%0A%0Avar%20x%20%3D%20new%20exampleClass()%20%3B%0Aconsole.log(await%20x.fun2())%3B%0A

Morkowski commented 7 years ago

Only second solution worked for me. use strict was added by babel-plugin-transform-es2015-modules-commonjs module included in es2015 preset.

I had to remove preset from .babelrc and configure plugins manually. My .babelrc before:

{
    "presets": ["es2015"],
    "plugins": [
        ["fast-async", { "compiler": { "noRuntime": true } }],
        "transform-class-properties",
        "transform-export-extensions",
        "transform-object-rest-spread",
        ["babel-plugin-module-alias", [{ "src": "lodash", "expose": "underscore" }]]
    ]
}

and after change:

{
    "plugins": [
        ["fast-async", { "compiler": { "noRuntime": true } }],
        "transform-class-properties",
        "transform-export-extensions",
        "transform-object-rest-spread",
        ["babel-plugin-module-alias", [{ "src": "lodash", "expose": "underscore" }]],

        //plugins from es2015 preset
        "babel-plugin-transform-es2015-template-literals",
        "babel-plugin-transform-es2015-literals",
        "babel-plugin-transform-es2015-function-name",
        "babel-plugin-transform-es2015-arrow-functions",
        "babel-plugin-transform-es2015-block-scoped-functions",
        "babel-plugin-transform-es2015-classes",
        "babel-plugin-transform-es2015-object-super",
        "babel-plugin-transform-es2015-shorthand-properties",
        "babel-plugin-transform-es2015-duplicate-keys",
        "babel-plugin-transform-es2015-computed-properties",
        "babel-plugin-transform-es2015-for-of",
        "babel-plugin-transform-es2015-sticky-regex",
        "babel-plugin-transform-es2015-unicode-regex",
        "babel-plugin-check-es2015-constants",
        "babel-plugin-transform-es2015-spread",
        "babel-plugin-transform-es2015-parameters",
        "babel-plugin-transform-es2015-destructuring",
        "babel-plugin-transform-es2015-block-scoping",
        "babel-plugin-transform-es2015-typeof-symbol",
        ["babel-plugin-transform-es2015-modules-commonjs", {"strict": false}]
    ]
}

Thanks @matAtWork for your help ;)

matAtWork commented 7 years ago

I'm glad you got it working - this is an interesting edge case that depends on a lot of variables (babel plugins, execution order, version of Safari), and I've not found a generic solution.

I'll close this, but feel free to update/re-open if you find out anything that might help resolve it in general.

techjoshua commented 7 years ago

I just encountered the same issue. I thought that I might be able to work around it by setting the es6target compiler option to true. While that did use arrow functions for the promise declarations it did not change the functions generated by the matchIfStmt. If those had become variables containing arrow functions I believe the babel es2015 preset defaults would have worked fine.

Also, while I haven't tried this, I think you can achieve a similar level of customization without manually re-adding all of the es2015 preset's plugins by doing something like this:

{
    "presets": [
        [ "es2015", { "modules": false } ]
    ],
    "plugins": [
        ["fast-async", { "compiler": { "noRuntime": true } }],
        ["babel-plugin-transform-es2015-modules-commonjs", {"strict": false}]
    ]
}

{ "modules": false } seems to omit any of the module transforms from the es2015 preset, leaving you free to configure the commonjs transform module yourself in the plugins section.

techjoshua commented 7 years ago

Also, here is an example where invalid code is being produced. ES6 modules are always in "strict" mode which does not permit functions to be defined. By using babel to transform ES6 modules to commonjs omitting the "use strict" we are working around the problem. However ES6 modules are very useful for webpack 2's "tree shaking" optimization and I'd prefer to not have to convert my code to commonjs in the first place. When I remove the commonjs transform plugin I get the following error with my code linked below:

PhantomJS 2.1.1 (Windows 8 0.0.0) ERROR
  SyntaxError: Functions cannot be declared in a nested block in strict mode

http://nodent.mailed.me.uk/#import%20getSavedDataList%20from%20'getSavedDataList'%3B%0Aimport%20getSavedData%20from%20'getSavedData'%3B%0A%0Aclass%20Test%20%7B%0A%20%20%20%20async%20getData()%20%7B%0A%20%20%20%20%20%20%20%20const%20result%20%3D%20%7BsavedData%3A%20null%2C%20savedTerm%3A%20''%7D%3B%0A%20%20%20%20%20%20%20%20const%20data%20%3D%20await%20getSavedDataList(this.havId%2C%20this.bizopsHost)%3B%0A%20%20%20%20%20%20%20%20if%20(data%20%26%26%20data.responseEntity%20%26%26%20data.responseEntity.summaries)%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20const%20searchData%20%3D%20data.responseEntity.summaries%5B0%5D%3B%0A%20%20%20%20%20%20%20%20%20%20%20%20if%20(searchData%20%26%26%20searchData.savedTerm)%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20result.savedTerm%20%3D%20searchData.savedTerm%3B%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20const%20savedData%20%3D%20await%20getSavedData(searchData.savedTerm)%3B%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20if%20(savedData)%20%7B%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20result.savedData%20%3D%20savedData%3B%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%7D%0A%20%20%20%20%20%20%20%20%20%20%20%20%7D%0A%20%20%20%20%20%20%20%20%7D%0A%0A%20%20%20%20%20%20%20%20return%20result%3B%0A%20%20%20%20%7D%0A%7D%0A%0Aexport%20default%20Test%3B~options~%7B%22mode%22%3A%22promises%22%2C%22promiseType%22%3A%22Zousan%22%2C%22noRuntime%22%3Atrue%2C%22es6target%22%3Atrue%2C%22wrapAwait%22%3Atrue%2C%22spec%22%3Afalse%7D

matAtWork commented 7 years ago

As discussed above, ES6 permits functions to be declared in block scope (see http://es6-features.org/#BlockScopedFunctions), not just at the top-level of a function, and so when using modules (e.g. webpack) nodent assumes (correctly) that the code is strict mode, and that block-scoped functions are permitted. Nodent (fast-async) in fact makes this assumption on the appearance of any ES6 keyword (like class).

Safari 9 on iOS does not comply with the ES6 standard, and rejects block-scoped functions in strict mode. In order to generate code that works on iOS, you need to remove all the use strict directives in the code generated by Babel. I believe this problem is common in code generated by Babel and targetting iOS - there is a plugin to do it (https://www.npmjs.com/package/babel-plugin-transform-remove-strict-mode). Note that both Babel and nodent generate code adhering to the specification in order to work correctly on other platforms (Chrome, Node, etc) - change the output for iOS Safari would break these spec-compliant platforms.

phyllisstein commented 7 years ago

Wanted to raise this again, as Webpack has recently thrown an additional monkey-wrench into the works. It now inserts a strict-mode directive into any modules that use ES6 imports or exports, which neither Babel nor any other build-pipeline hack can reach and remove. I seem to recall the spec option fixing the issue on iOS <=9 in the past, but if that was once a viable solution it no longer seems to be.

I understand and buy the argument that iOS Safari is actually in the wrong here, and I even understand and buy the point that it's a big fix to accommodate a browser with 0.58% global usage. All the same, if there's a workaround you could recommend other than disabling strict mode I'd be eager to hear it.

MatAtBread commented 6 years ago

Short answer - I don't have a solution. One mechanism could be to use https://github.com/matAtWork/nodent-loader, which applies nodent before webpack gets a chance to mess with your code. I only really have one project that relies on webpack and nodent, and this works for me.

MatAtBread commented 6 years ago

I've just spotted that https://github.com/matAtWork/nodent-loader hasn't been updated since Nodent v2. I know it works with v3, but you might have to hack the package.json

phyllisstein commented 6 years ago

Thanks for your feedback! I will absolutely look into that. (Or just drop the somewhat quixotic hope of supporting iOS9, hah.)