GitHub PullRequestReviewCommentEvent at 2014-04-23 05:54:27
Repository: qunit
Actor: Krinkle
Payload: {"comment":{"url":"https://api.github.com/repos/jquery/qunit/pulls/comments/11887505","id":11887505,"diff_hunk":"@@ -0,0 +1,66 @@\n+/* jshint node:true */\n+/* global define:true */\n+;( function() {\n+\n+\tvar root = typeof global === \"object\" && global || this,\n+\t\tload = ( typeof require === \"function\" && !(root.define && define.amd) ) ? require : ( !root.document && root.java && root.load ) || Function.prototype,","path":"test/env-tests.js","position":6,"original_position":6,"commit_id":"5ffb54d70fd3d672f613ddc3c6da98f0c94e895b","original_commit_id":"5ffb54d70fd3d672f613ddc3c6da98f0c94e895b","user":{"login":"Krinkle","id":156867,"avatar_url":"https://avatars.githubusercontent.com/u/156867?","gravatar_id":"49061e8503ca321a8045c9065963a8c6","url":"https://api.github.com/users/Krinkle","html_url":"https://github.com/Krinkle","followers_url":"https://api.github.com/users/Krinkle/followers","following_url":"https://api.github.com/users/Krinkle/following{/other_user}","gists_url":"https://api.github.com/users/Krinkle/gists{/gist_id}","starred_url":"https://api.github.com/users/Krinkle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Krinkle/subscriptions","organizations_url":"https://api.github.com/users/Krinkle/orgs","repos_url":"https://api.github.com/users/Krinkle/repos","events_url":"https://api.github.com/users/Krinkle/events{/privacy}","received_events_url":"https://api.github.com/users/Krinkle/received_events","type":"User","site_admin":false},"body":"Hm.. Relying on `Function.prototype` being a function itself (`function Empty() {}`). I thought that was a proprietary thing that some browsers did and kept for legacy reasons. I vague remember seeing it once while debugging.\r\n\r\nI would've never thought it was actually usable as a function. Looks like this actually made it into the spec as early as ES3, and renewed in ES5 for the long run.\r\n\r\n[ECMAScript 5.1](http://es5.github.io/#x15.3.4) / [ECMAScript 3](http://www.ecma-international.org/publications/files/ECMA-ST-ARCH/ECMA-262,%203rd%20edition,%20December%201999.pdf#page-86)\r\n> 15.3.4 Properties of the Function Prototype Object\r\n> The Function prototype object is itself a Function object (its [[Class]] is \"Function\") that, when invoked, accepts any arguments and returns undefined.\u0000\u0000\r\n\r\nI wouldn't be surprised if some older browsers never implemented on this, but let's say for now that it's reliable.\r\n\r\nIn all these years, I've never once run into code that invokes the Empty built-in prototype function and uses it as a built-in noop. Maybe just use `function () {}` here, for sake of readability? The code looks quite counter-intuitive.\r\n\r\nOr maybe this is a hidden gem that we should adopt more widely?","created_at":"2014-04-23T05:54:27Z","updated_at":"2014-04-23T05:54:27Z","html_url":"https://github.com/jquery/qunit/pull/540#discussion_r11887505","pull_request_url":"https://api.github.com/repos/jquery/qunit/pulls/540","_links":{"self":{"href":"https://api.github.com/repos/jquery/qunit/pulls/comments/11887505"},"html":{"href":"https://github.com/jquery/qunit/pull/540#discussion_r11887505"},"pull_request":{"href":"https://api.github.com/repos/jquery/qunit/pulls/540"}}}}