bestiejs / json3

A JSON polyfill. No longer maintained.
https://bestiejs.github.io/json3
Other
1.02k stars 150 forks source link

Update has() to cache results and not repeat tests #40

Closed bnjmnt4n closed 10 years ago

bnjmnt4n commented 10 years ago

I've switched has to an IIFE returning an object with support info. This prevents tests from being repeated as well, ie.

if (!has("json")) {
  if (!has("json-stringify")) {
    // ...
  }
  if (!has("json-parse")) {
    // ...
  }
}

repeats the json stringify and parse support tests twice each.

ghost commented 10 years ago

I used the has("...") form for compatibility with has.js-aware optimizers, like RequireJS. The optimization tool will replace the has call with a Boolean primitive, and unnecessary branches (along with the has function) can then be removed during minification. I think this pull request would break that behavior, though—this is why the has function is self-contained. +@unscriptable @jaredcacurak

I could store the test results as properties on the has function itself, though—that shouldn't interfere with Uglify's or Closure Compiler's dead code removal. I remember that the has function wouldn't be removed if I stored the test results in closed-over outer variables; this may no longer be the case. I'll have a look.

bnjmnt4n commented 10 years ago

Thanks anyway, I might just rewrite the tests to cache the results on has when I'm free.

unscriptable commented 10 years ago

Yah, I don't like that the has(id) syntax requires strings in the code. Seems so primitive. Anybody want to write a has2.js? ;)

bnjmnt4n commented 10 years ago

Will close this for now and open new PR

bnjmnt4n commented 10 years ago

Now at #41