Using Array.isArray when it should be dust.isArray #210

Closed
SomethingSexy opened this Issue Dec 19, 2012 · 7 comments

Projects

None yet

5 participants

@SomethingSexy

It looks like in the full version, there is still one case where you are still accessing Array.isArray (at line 943 of dust-full-1.1.1) instead of your internal dust version of isArray.

This normally is not a problem but I tried running the compiler via Rhino and this was causing an issue. Switching it over to reference dust.isArray fixed the issue.

@vybs

interesting, can you share what was the exception or error?

We use it at LI a lot and did not yet run into this.

@SomethingSexy

Yea definitely!

We are using Rhino 1.7R2.

I am running it pretty standard, loading up Rhino and then adding dust and calling the compile method on my templates. If I run it against the 1.1.1 version of dust with the single Array.isArray call in there I get the following exception:

Exception in thread "main" java.lang.RuntimeException: org.mozilla.javascript.JavaScriptException: TypeError: Cannot find function isArray in object function Array() { [native code for Array.Array, arity=1] }

When I update it to dust.isArray all is good.

@SomethingSexy

Of course!

Running the compiling through Rhino we were receiving the following exception:

Exception in thread "main" java.lang.RuntimeException: org.mozilla.javascript.JavaScriptException: TypeError: Cannot find function isArray in object function Array() { [native code for Array.Array, arity=1] }

Once we updated to be dust.isArray everything worked.

@rragan

Likely that the version of Rhino being run lacks the Array.isArray feature added in ECMAScript 5. Likely a newer version of Rhino would get you past the problem for now: https://developer.mozilla.org/en-US/docs/New_in_Rhino_1.7R3

Which raises the more general question: What browser version/JavaScript versions are going to be supported with dust?

@jairodemorais

We should support the maximun number of browsers I think.

Are we going to change the code, I mean replace the Array.isArray by dust.isArray in this case? I could make that little change so we can close this issue.

@promano-urbandaddy

This also seems to effect IE<9

@paulogaspar7 paulogaspar7 pushed a commit to paulogaspar7/dustjs that referenced this issue Feb 18, 2013
Paulo Gaspar Fix for lnkedin/dustjs issue #210. All tests passed. f5850ad
@vybs

thanks. good to merge.
@jairodemorais @rragan for 2 more oks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment