search

umdjs / umd

UMD (Universal Module Definition) patterns for JavaScript modules that work everywhere.
MIT License
7.42k stars 423 forks source link

Cosmetic changes #54

Closed janpaepke closed 9 years ago

janpaepke commented 9 years ago

Hi Guys. I simplified the no-dependency function definition, to save a total of 3 chars (wohoo!!) Also made some changes to sync returnExports.js and returnExportsGlobal.js. See DIFFs for details.

jrburke commented 9 years ago

The empty dependency array is there to prevent Function.toString() scanning of the factory for dependencies. This is particularly important if the factory is made up of internal modules that should not be visible to the outside, and saves some computation work.

I have not looked in depth at the rest of it, but in general, I suggest creating new files instead of putting multiple examples in a file.

janpaepke commented 9 years ago

Hi jburke,

thanks for the clarification. let's keep it in there then.

I added the "no dependency" example to returnExportsGlobal.js because returnExports.js also contains one. So the Idea was to sync them.

I could revise the pull request to A - include the extended returnExportsGlobal.js or B - just correct typo and sync wording.

How do you want me to proceed?

janpaepke commented 9 years ago

@jrburke bump

jrburke commented 9 years ago

Sorry for the delay in response. Keeping the format the same as returnExports.js seems fine. It has been a while since I actively participated to the recipes, so not sure of their full current state, sorry I gave bad initial advice for that part.

I mostly watch for issues for the AMD parts, as others seem to be able to handle the other parts of merging and deciding what goes in this repo, and I think what has been added to this repo has been nice, but outside the bare minimum I thought was needed, just showing the basic way to do it.

There are lots of variations possible, and I did not think we needed to document them all, as it can get overwhelming, by giving too many choices. The tricky part is that people have different styles they prefer. I just mostly wanted to see a base set I know would work. Perhaps other people's interest in tests would help that matter.

For me though, I am not actively scrubbing issues/pull requests here, mostly just skim for things I know would adversely affect AMD loaders.

janpaepke commented 9 years ago

So how do you suggest we proceed here?

EvanCarroll commented 9 years ago

Damn, this should have been committed when it was clean.

EvanCarroll commented 9 years ago

I'm now a maintainer @janpaepke and I would have committed this patch. Let's talk about getting this in, FB or gTalk me, me at evancarroll dot com.