Added a block to support AMD module loading

[bringking]

Ran into a problem loading this in my project. Added a block to support an AMD environment.

[acornejo]

I tested this pull request and it works great, its just 3 lines of code, please add them to allow amd support.

[SteveSanderson]

Thanks for this! I'll add the AMD loader support early next week.

On Friday, December 27, 2013, Charles King notifications@github.com wrote:

Ran into a problem loading this in my project. Added a block to support an

AMD environment.

You can merge this Pull Request by running

git pull https://github.com/bringking/knockout-projections master

Or view, comment on, or merge it at:

https://github.com/SteveSanderson/knockout-projections/pull/8 Commit Summary

• added a block for amd module scenarios
• Update knockout-projections.js

File Changes

• M src/knockout-projections.jshttps://github.com/SteveSanderson/knockout-projections/pull/8/files#diff-0(6)

Patch Links:

• https://github.com/SteveSanderson/knockout-projections/pull/8.patch
• https://github.com/SteveSanderson/knockout-projections/pull/8.diff

[gambrose]

I believe calling require is wrong as this does no guarantee that ko has been extended when it is used as a dependency. The require callback can be called after dependant module has loaded.

Using define instead and returning an object, probably ko, as done in common js implementation would be preferable.

[acornejo]

Hi @gambrose, I don't quite understand your comment (maybe a grammar issue?), but I think I know what you mean.

Personally I would do it like this:

  if (typeof module !== 'undefined' && typeof module.exports !== 'undefined')                                                                                               
    attachToKo(require('ko'))
  else if (typeof define === 'function' && define.amd)
    define(['knockout'], attachToKo);
  else if ('ko' in global)
    attachToKo(global.ko);

Which is very readable and correct (I use that pattern in every client side library I've written).

[bringking]

@SteveSanderson I would disregard my PR since @acornejo's solution is the correct way to define a module with a dependency using AMD. I sometimes forget and mix the two.. Apologize about that.

[gambrose]

@acornejo here is a scenario to try and better explain:

myModule depends on knockout and knockout projections

These modules can be loaded in the following order: knockout, myModule, knockout projections. This will probably result in myModule erroring with no map or filter function defined on observable array.

What you want is knockout projections to always be loaded before myModule is loaded which is what you get if you use define instead or require.

[acornejo]

I think we might be splitting hairs here, but here is why I don't see the problem with your scenario.

If requirejs loads knockout-projections BEFORE loading knockout, then it will see the require(["knockout"], blabla) call, at which point it will load knockout before to continue loading knockout-projections, and everything will be fine.

The reason why it is better practice to use define instead of require is because that way the ordering can be decided at compilation time, and requirejs will optimize the ordering of the scripts to that effect. require should be used in cases where the dependencies are not known at runtime.

For instance, this would be a good use case for using require

define(['dependency', function (dep) {
    if (dep.flag1)
       require(['jquery'], dostuff);
    else
       require(['sizzle'], dostuff);
});

This dynamic choice of which modules to load cannot be accomplished by define calls. So the rule of thumb, use define everywhere possible, use require only when you must.