> On 2012-01-03 08:45:27, Paul Lindner wrote:
> > I'm okay with the patch as-is, but would prefer something like this which
> > is a modified version of the closure library goog.isDef
> >
> >
> > goog.isDef = function(val) {
> > var undefined;
> > return val !== undefined;
> > };
> >
>
> Dan Dumont wrote:
> Paul, is google's deployment running with advanced optimizations turned
> on?
>
> Stanton Sievers wrote:
> Paul, where would such a definition live such that it is accessible
> across all of the JavaScript files? Any idea how we could hook that up? I'd
> be up for trying that... although, if we were going to introduce goog.isDef I
> would like to replace all typeof instances with that, which is going to be a
> much bigger change.
maybe add it to core.util? Or possibly have its own feature which you add as a
dependency to core...
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3332/#review4174
-----------------------------------------------------------
On 2011-12-29 20:05:42, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3332/
> -----------------------------------------------------------
>
> (Updated 2011-12-29 20:05:42)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> The purpose of this patch is to remove equality checking against the
> "undefined" keyword, i.e., "foo != undefined". I've replaced such instances
> with "typeof foo != 'undefined'" instead. This eliminates possible errors
> that can arise if the "undefined" keyword is reassigned.
>
> A little background regarding the motivation for this patch.... As of
> ECMAScript 3 and before ECMAScript 5, undefined is a keyword but is not
> restricted, thus, it's value can be re-assigned. This is generally caused by
> a programming error, such as assigning values to object attributes without
> first checking that the object attribute exists - making them undefined.
> When these types of errors occur they are difficult to track down and debug.
> This patch is one way to help reduce possible errors caused by undefined
> being re-assigned.
>
> To find instances to replace, I searched for this regular expression in the
> code base using Eclipse search within *.js files: =[ ]*undefined[ ]*
>
> I ignored certain files, such as external libs (CodeMirror, etc) and most
> tests.
>
>
> Diffs
> -----
>
>
> http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/cconviews.js
> 1225624
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js
> 1225624
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js
> 1225624
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.json/json-flatten.js
> 1225624
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/i18n/datetimeparse.js
> 1225624
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/open-views/viewenhancements-container.js
> 1225624
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shindig.container/shindig-container.js
> 1225624
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shindig.uri/uri.js
> 1225624
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/osapi/batchtest.js
> 1225624
>
> Diff: https://reviews.apache.org/r/3332/diff
>
>
> Testing
> -------
>
> Built everything. Ensured that the common container sample gadgets still
> render.
>
>
> Thanks,
>
> Stanton
>
>
