Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[esnext] “[强制] 不得在顶层作用域下使用箭头函数”此条是否过于严苛? #19

Closed
hax opened this issue Nov 10, 2015 · 15 comments

Comments

@hax
Copy link

hax commented Nov 10, 2015

看内容主要是this的问题,但是arrow function也可能并没有引用this。这个时候并没有什么问题。

例子:

window.addEventListener('load', _ => console.log('loaded!'))
@errorrik
Copy link
Contributor

@otakustay 这条是你定的。你倾向限制,还是让开发者自己决定,当我要用caller给我的this时,我自己不写箭头函数?

@hax
Copy link
Author

hax commented Nov 11, 2015

我个人认为禁止绑定了top-level的this的函数(无论是否是arrow function)是合理的。但是eslint暂时还没有这样的rule。

@otakustay
Copy link
Member

@hax 在首楼里的示例并不在我的“顶层”的范围内,这里确实是文字上没有说清楚,我指的是

let inc = i => i + 1;

let foo = inc(3);

这样的代码应该写成

function inc(i) {
    return i + 1;
}

var foo = inc(3);

当然第一段代码没错,但是顶层函数确实不应该绑定this,且顶层函数多数是工具类型的函数,我不希望造成一种“这个函数会指定this”或者“这是个lambda类的管道型函数”(这个表达太晦涩)这样的错觉

@hax
Copy link
Author

hax commented Nov 11, 2015

因为之前有一种风格是:方法用缩写,需要this动态绑定的函数(如用于bind operator)用function,其他(包括所有没有this和不需要递归的)用arrow function。按这种风格会更多使用arrow function。

const inc = i => i + 1

function inc(i) {
  return i + 1
}

比较如下:

arrow function在这种use case的缺点:

  1. 连续的等号和箭头号,代码略难辨识
  2. 必须写在调用处之前(而普通函数可以写在后面)
  3. 并没有绑定lexical this的需要

所以我现在考虑倾向于使用eslint的http://eslint.org/docs/rules/prefer-arrow-callback 。完整的规则表述应该是:

如是不包含this的callback,或包含this并且跟随.bind(this)的callback,且不是generator、递归的场景,应使用arrow function。

此规则的唯一问题是:

method() {
  x.addEventListener('click', function () { this.flag = true })
}

这样的代码很可能是写错了——忘记用.bind(this)或者arrow function,但是现有的规则无法发现这个潜在的错误。
我们只能寄望引入其他检查,比如当有类型标注时,或许可以有类型属性不匹配的警告。

@errorrik
Copy link
Contributor

如是不包含this的callback,或包含this并且跟随.bind(this)的callback,且不是generator、递归的场景,应使用arrow function。

这条规则我觉得复杂到不太容易被理解和记忆。另外,我们依然需要一条额外的规则,去规定不要使用arrow function去绑定top-level的this

@hax
Copy link
Author

hax commented Nov 16, 2015

@errorrik
我描述的是ESLint这条prefer-arrow-callback规则的完整行为。但是代码风格本身其实就很简单的说“callback应该优先使用arrow function”即可。

完整描述之所以看起来复杂是因为它详细描述了所有不适用场景。但是使用时其实通常并不需要考虑,比如generator根本没有arrow function写法,也不是callback;又如递归,你用arrow function写不出递归,自然没法直接用。

至于top-level的规则,其实eslint有另一条 http://eslint.org/docs/rules/no-invalid-this 包含了这个检测。

@otakustay
Copy link
Member

补充下最近的研究:

在当前babel的实现下,在顶层使用async配合箭头函数会产生不符预期的结果

代码如下:

'use strict'

let foo = async (x, {value = 1} = {}) => x + value

foo(1).then(i => console.log(i))
foo(1, {value: 2}).then(i => console.log(i))

使用babel-node运行,得到的输出如下:

2
2

这显然是不合理的,我们期望第2次调用输出的结果为3

进一步看babel编译的代码:

'use strict';

var _arguments = arguments,
    _this = this;

var foo = function foo(x) {
  var _ref,
      _ref$value,
      value,
      args$1$0 = _arguments;

  return regeneratorRuntime.async(function foo$(context$1$0) {
    while (1) switch (context$1$0.prev = context$1$0.next) {
      case 0:
        _ref = args$1$0.length <= 1 || args$1$0[1] === undefined ? {} : args$1$0[1];
        _ref$value = _ref.value;
        value = _ref$value === undefined ? 1 : _ref$value;
        return context$1$0.abrupt('return', x + value);

      case 4:
      case 'end':
        return context$1$0.stop();
    }
  }, null, _this);
};

foo(1).then(function (i) {
  return console.log(i);
});
foo(1, { value: 2 }).then(function (i) {
  return console.log(i);
});

问题在于因为async的关系,babel的arguments处理产生了错误,而函数默认值依赖arguments

把代码中的async去除,bable能得到正确的结果:

'use strict';

var foo = function foo(x) {
  var _ref = arguments.length <= 1 || arguments[1] === undefined ? {} : arguments[1];

  var _ref$value = _ref.value;
  var value = _ref$value === undefined ? 1 : _ref$value;
  return x + value;
};

foo(1).then(function (i) {
  return console.log(i);
});
foo(1, { value: 2 }).then(function (i) {
  return console.log(i);
});

把箭头函数改为普通的function,也同样能得到正确的结果

'use strict';

var foo = function foo(x) {
    var _ref,
        _ref$value,
        value,
        args$1$0 = arguments;

    return regeneratorRuntime.async(function foo$(context$1$0) {
        while (1) switch (context$1$0.prev = context$1$0.next) {
            case 0:
                _ref = args$1$0.length <= 1 || args$1$0[1] === undefined ? {} : args$1$0[1];
                _ref$value = _ref.value;
                value = _ref$value === undefined ? 1 : _ref$value;
                return context$1$0.abrupt('return', x + value);

            case 4:
            case 'end':
                return context$1$0.stop();
        }
    }, null, this);
};

foo(1).then(function (i) {
    return console.log(i);
});
foo(1, { value: 2 }).then(function (i) {
    return console.log(i);
});

在没有继续详细对照箭头函数的规范的当前,我倾向于认为这是babel的BUG

但babel是我们现在使用ES Next必备的工具,所以规范为其作出妥协也情有可原

因此我建议在当下我们避免在顶层使用箭头函数,至少避免使用async的箭头函数

@otakustay
Copy link
Member

留一个Babel对应的Issue:https://phabricator.babeljs.io/T6747

@hax
Copy link
Author

hax commented Dec 3, 2015

这个是bug。

像bug这种东西,我认为应该跟代码标准分开来。代码标准是基本稳定不变的东西,而bug列表则是很快会变化的。

另外这个bug跟是不是在顶层没有关系啊。

@hax
Copy link
Author

hax commented Dec 3, 2015

我刚刚试验了下,其实单独regenerator或async-to-generator都不会有这个bug。

以下情况会出现此bug:

  1. 同时使用regenerator和async-to-generator(注意后者被默认包含在preset stage-3里,而stage-0到stage-2都自动包含了stage-3,而preset es2015默认包含了regenerator)
  2. plugins的顺序上async-to-generator在parameters之后(注意presets永远排在单独的plugins之前,导致 --presets es2015 --plugins transform-async-to-generator会挂掉)

所以现在大家常用的配置正好会产生bug,唔!不过可以通过配置文件调整来规避。

@otakustay
Copy link
Member

这个问题babel 5.x同样存在,所以可以不管presets和plugins等配置

昨天太晚是我疏忽了,显然不是顶层作用域同样会出事,只是发现问题的时候正好在node的模块作用域里,而node的模块作用域的arguments是有内容的,导致花了不少时间排查解决这个事……

@errorrik 我们的编码规范本身起到一定的指导和实践的作用,因此对async和箭头函数混用的情况我们是不是临时给出一个建议,在babel修复并稳定后再移除此条?

@otakustay
Copy link
Member

另外经过一周时间的实践后,原则上我支持顶层也使用箭头函数

不过我确实挺担心箭头函数用得太狠是否会对普通的开发者产生阅读上的困难

exports.shortcut = (browsingContext, surface) => (key, command) => {
    document.addEventListener(
        'keyup',
        e => {
            if (browsingContext.isLocked) {
                return
            }

            if (e.keyCode === key) {
                command(browsingContext, surface)
            }
        },
        false
    )
}

在我看来,当箭头函数用作函数工厂的形式时,甚至有更多层的工厂的场景下,既漂亮(对习惯了的人来说)又令人担心可读性(对更多不习惯的人来说)……

@errorrik
Copy link
Contributor

@errorrik 我们的编码规范本身起到一定的指导和实践的作用,因此对async和箭头函数混用的情况我们是不是临时给出一个建议,在babel修复并稳定后再移除此条?

如果要建议,也只能在环境章节。不过,我觉得,这个显而易见的坑应该不会存在很长时间,可以不用建议。(如果一个条目存在时间不超过3个月,就没必要存在了)

@errorrik
Copy link
Contributor

更多讨论和当下的结论在 #28 ,so close this issue

@errorrik
Copy link
Contributor

对于箭头函数,当前的实践不足以得出哪种做法更make sense的结论,所以,暂时不对箭头函数的使用做任何限制。

删除[强制] 不得在顶层作用域下使用箭头函数条目

erik168 added a commit that referenced this issue Dec 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants