jneen / pjs

Classes for javascript that don't suck.
MIT License
195 stars 29 forks source link

AMD pattern #3

Closed lukemorton closed 12 years ago

lukemorton commented 12 years ago

It would be nice if you could support the AMD pattern as well as the CommonJS module pattern so that I can load this file using require.js or curl.js client side.

Take the example of Klass for example: https://github.com/ded/klass/blob/master/klass.js

Dustin does this:

!function (name, definition) {
  if (typeof define == 'function') define(definition)
  else if (typeof module != 'undefined') module.exports = definition()
  else this[name] = definition()
}('klass', function () {

  // Define Klass in here

    return Klass;
})

More info on AMD: https://github.com/amdjs/amdjs-api/wiki/AMD

jneen commented 12 years ago

Hm. I wonder if there's a way to do this where the minified size is still < 500b. Perhaps a way to make a specialized p.amd.js and a p.common.js that export it in different ways? Or maybe that's too much and I should just put the boilerplate up at the top...

jneen commented 12 years ago

What do you think? I'm not sure I got the AMD pattern right, but since P has no dependencies, I believe I can just do the define call afterwards, right?

jneen commented 12 years ago

Alright, I'm pretty satisfied with the outcome. Run make amd and you'll have p.amd.js and p.amd.min.js available in the build dir. Same with make commonjs.

What I recommend, though, is that since the lib is so small, you may just want to concatenate or inline it with some other JS files. The default p.min.js just sets var P, so that for libraries (like mathquill, which this was originally written for), you can inline it within a closure, and not pollute the end-user's namespace.

lukemorton commented 12 years ago

Thanks for your work on this so far! There is a problem however, define() is better without 'pjs' defined and it should be a function that returns the P object like so:

define(function () { return P; });

However you are still defining P in global scope within the browser environment so even if you fix p.amd.js there is still that problem.

I understand what us developers are like wanting to keep a file size down, and make boilerplate as minimal as possible so I can appreciate what you did with the Makefile.

I have half fixed the issue although I missed out on fixing the Makefile due to lack of knowledge :P

See #4