> 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?
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.
- Stanton
-----------------------------------------------------------
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
>
>
