Update has.js

[AndrewEastwood]

Adding requirejs config support

[jdalton]

lets match the style:
==, no space between the if and the (, and use brackets {}.

[AndrewEastwood]

It's ok to remove spaces but why do you want to use non-strict comparation for that? eqeqeq works faster and typeof always returns string

[jdalton]

This code going to be hit a few million times is it? Naw, it's a micro-opt and counter to the existing style. Technically because typeof always returns a string the == is performing the same steps as ===.

[AndrewEastwood]

ok, I removed third '='

[jdalton]

The code style seems to favor hoisting vars out (it was a phase :P). So that would make it:

var propKey, moduleConfig;
if(typeof module.config == "function"){
  moduleConfig = module.config();
}
for(propKey in moduleConfig){

[AndrewEastwood]

What if we just use simple code for this as @jrburke has done in https://github.com/requirejs/text/blob/master/text.js#L23

var moduleConfig = (module.config && module.config()) || {};
for(var propKey in moduleConfig){
    has.add(propKey, moduleConfig[propKey]);
}

something like that above

[jdalton]

Naw, different project styles. We are consuming a foreign thing so have to be more cautious whereas that plugin expects to work with RequireJS and its constructs.

[jdalton]

Soo close! Brackets + spacing of the for(

[AndrewEastwood]

😄 what about now?

[jdalton]

Can you add a unit test and have you signed the CLA?

[jdalton]

no need to init moduleConfig = {}; it can just be moduleConfig;.

[AndrewEastwood]

alright. I've signed it just now 😀

[jdalton]

CLA submitted on 2014-06-05 21:06:04.

[jdalton]

So for the test it looks like runTests.html has require.js commented out. Maybe enable it below the other script loads, load it via AMD+config, & delay the population of the results until after the module is loaded. Whew.

[AndrewEastwood]

omg! the version of requirejs is 0.14.5+
I'm gonna upgrade it to 2.1.14

[fn-zz]

+1

[AndrewEastwood]

I come up with that approach for tests or, we can rewrite all detects to use requirejs (:

[jdalton]

I'll try to review this soon but can't make any promises 😞

[AndrewEastwood]

This waits whole month for review 😒

[jdalton]

Sorry, I haven't made time for this yet.

[AndrewEastwood]

Howdy @jdalton! So, what's about this?

[jdalton]

I gotta be honest, I'm not really an active contributor to this project anymore. If any other contributor has time to pick this up, review, & merge please have at it.

[AndrewEastwood]

I see. Alright, let's put this on their decisions :)