fix(typo): should pass exports to factory when amd

[xujihui1985]

is this a typo? if it is amd, we should pass exports object to factory instead of root, isn't it?

[ConnorAtherton]

Won't root be the global Window object in this case, and at the bottom of the file we attach Walkway to the global object, named exports in the factory function. I don't see a problem with this, it's late right now but I will double check tomorrow.

Do you have a test case you could share that proves this doesn't work for you?

[xujihui1985]

there is no problem to use root aka window as the export in this case, however, the purpose to use this pattern is to adapt for amd pattern or commonjs pattern.

require(['./walkway'], function(walkway) {
    console.log(walkway);
});

if I want to use your code with requirejs, and use walkway as a dependence of another module, then I cann't get the object if you export the object to root, because in this case root !== exports

[jeffcarp]

Hey @xujihui1985, I added the UMD declaration by following these examples: https://github.com/umdjs/umd

I'm not very familiar with how AMD works, so it may very well have been a typo on my end.

[xujihui1985]

Hi @jeffcarp,

;(function (root, factory) {
  if (typeof define === 'function' && define.amd) {
    // AMD. Register as an anonymous module.
    define(factory);
  } else if (typeof exports === 'object') {
    // CommonJS
    module.exports = factory();
  } else {
    // Browser globals
    root.Walkway = factory();
  }
}(this, function factory () {
  'use strict';
...
...

return Walkway;
}));

please consider use this pattern, because the object you returned is actually a constructor, so

in requirejs.

require(['./walkway'], function(Walkway) {
   var svg = new Walkway('#triangle');
   ...
});

or in commonjs / bowserify

var Walkway = require('./walkway');

 var svg = new Walkway('#triangle');
...

in browser

var svg = new Walkway('#triangle');

[jeffcarp]

Hey @xujihui1985, sorry for the late reply. I tested Walkway out with RequireJS and verified that it is broken and that this PR fixes the problem, so it should be good to merge (cc/@ConnorAtherton).

Your second suggestion looks cool, is there a reason the suggested UMD code isn't structured that way?

[ConnorAtherton]

Fixed in latest version :+1:

[xujihui1985]

@ConnorAtherton great, thanks for take my suggestion.