Hello, This morning I found some code that I thought ESLint should warn about. I started to submit a GitHub issue, but I found an existing issue for the same thing. The issue is still open but the comments on it make it sound like it's an undesired feature. I read all of the discussion and I disagree with some of the conclusions which were drawn. I don't want to be a pain by arguing on old issues or creating duplicate issues so I hope this mailing list is a more appropriate forum to have this discussion.
Here is the reasoning that leads me to this post: Issues occasionally come up with code like: someFunction.apply(thisArg, args); someFunction might not inherit from Function.prototype or someFunction might have its own apply property. The workaround has been to use: Function.prototype.apply.call(someFunction, thisArg, args); As of ECMAScript 2015 there is a simpler workaround: Reflect.apply(someFunction, thisArg, args); Since Reflect.apply avoids the issues and is less verbose, it makes sense to me to always use Reflect.apply and to never use Function.prototype.apply or Function.prototype.call. According to the MDN documentation <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/apply#Description>, this appears to be the exact purpose of Reflect.apply. It would be great if ESLint supported this idea. The prefer-reflect rule implemented this, it would warn on use of Function.prototype.apply or Function.prototype.call, but this is not the only thing the prefer-reflect rule did. It also warned on use of the delete keyword and a handful of Object methods. After some discussion the entire rule was eventually deprecated and is to be removed because it was determined that the Reflect methods are not actually intended to replace the Object methods. I agree with the reasoning for removing prefer-reflect but I also agree with this comment from corbinu <https://github.com/eslint/eslint/issues/7226#issuecomment-257108713>: "However it does appear that Reflect.apply is not a companion for an Object method but meant to be "less verbose and easier to understand" than Function.prototype.apply.call. I was wondering if you would be open to the addition of a new rule that is just prefer-reflect-apply." Further comments recommend implementing a prefer-reflect-apply rule as a custom plugin because by itself its scope is too small for a core rule I would like to challenge that argument. From a personal and non-scientific perspective, the scope seems huge. There are some core rules that have conditions that I have never even seen come up, whereas I see Function.prototype.apply used very frequently. The prefer-spread rule should handle cases where Reflect.apply calls could be rewritten with spread syntax. This is actually what I ran into this morning. I had some code like: Reflect.apply(someObject.someMethod, someObject, args); and I realized it would be better written as: someObject.someMethod(...args); and I was looking to see if ESLint had a rule to check for this. I found an existing issue requesting this feature: https://github.com/eslint/eslint/issues/7401 The discussion on this issue doesn't make sense to me. First it was recommended to use no-restricted-properties to disallow the use of Reflect.apply. Since disallowing Reflect.apply altogether solves the initial issue, it was decided to use this as a workaround and not update this rule. That solution doesn't solve the issue for anyone who wants to use Reflect.apply. That brings me to the big question. Should I really be wanting to use Reflect.apply everywhere in my code? Is there a flaw in my reasoning above? In the same issue nzakas commented <https://github.com/eslint/eslint/issues/7401#issuecomment-255483850>, "I don't think Reflect.apply() is the same case, as I'm assuming people will mostly use that inside of proxies or will have other reasons for using it." This comment brings up more questions. What does Reflect.apply have to do with proxies? What are the other reasons for using it? Then I still don't agree with the conclusion. Even if I'm wrong and Reflect.apply shouldn't be used always, if there are valid use cases in proxies or for 'other reasons', shouldn't this rule still help with Reflect.apply in those cases? I agree with nzakas that "It seems unlikely that someone would use Reflect.apply(someFunc, null, args) instead of someFunc.apply(null, args) accidentally." but that misses the entire point of the prefer-spread rule. It does seem likely that someone would use Reflect.apply(someFunc, null, args) instead of someFunc(...args) accidentally. The no-useless-call rule should also handle useless Reflect.apply calls. I haven't seen this one brought up before but it's the same basic thing. Reflect.apply(someFunction, null, []) makes sense to rewrite as someFunction(). I apologize this turned into a bit of a rant. I started using Reflect.apply when the prefer-reflect rule started warning about it. It actually helped me find and fix a couple bugs so I was excited to go through and update all my code. Now I'm disappointed because ESLint isn't supporting Reflect.apply as well as it supports Function.prototype methods and there seems to be resistance to including Reflect.apply support. I'm not sure what I'm missing. I'll write a custom plugin if I need to but I feel like this should be a big enough deal to have in core. Thanks, Steven -- You received this message because you are subscribed to the Google Groups "ESLint" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
