Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMD pattern #3

Closed
lukemorton opened this issue Jan 4, 2012 · 4 comments
Closed

AMD pattern #3

lukemorton opened this issue Jan 4, 2012 · 4 comments

Comments

@lukemorton
Copy link
Contributor

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
Copy link
Owner

jneen commented Jan 4, 2012

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
Copy link
Owner

jneen commented Jan 5, 2012

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 added a commit that referenced this issue Jan 5, 2012
jneen added a commit that referenced this issue Jan 5, 2012
@jneen jneen closed this as completed in 1ba446a Jan 5, 2012
@jneen
Copy link
Owner

jneen commented Jan 5, 2012

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 lukemorton mentioned this issue Jan 5, 2012
@lukemorton
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants