UMD Wrapper

[kirill-konshin]

For issue #29

[dcousens]

@kirill-konshin this should be solved at a different level.

I'm not familiar with this wrapper, but is there any reason you couldn't use a tool like: https://www.npmjs.com/package/gulp-wrap-umd ?

These export 'styles' should be agnostic of the code itself.

My understanding for

if (typeof module !== 'undefined' && module.exports) {

Is simply because the repository supports bower and doesn't have a pre-built dist.

[kirill-konshin]

Of course it could be a part of build procedure, but it's not yet implemented, and for such a small module it's an overkill. It's easier to just wrap it once manually.

[dcousens]

@kirill-konshin except it pollutes a simplistic module with export boilerplate.

[kirill-konshin]

@dcousens aha ;) but without this "pollution" module is unusable with RequireJS (okay, usable, but requires a shim configuration, which is not good).

Choose what has more value: super-clean module or better compatibility.

[dcousens]

IMHO, super clean module. I'll pass this over to @JedWatson

[kirill-konshin]

I strongly disagree. Clean and straightforward install/usage procedure is more important. What is the reason the one creates a module, for the sake of clean code and ego-boost or to make consumers' life easier? I doubt anybody would want to see the source if module works well, has good docs and works everywhere.

Plus, UMD does not pollute the code that much ;)

[dcousens]

Coming from a security background, simplicity of implementation trumps any convenience hack(s) added for edge cases.

I see this as a problem with your build system, personally, if this was my issue, I'd separate the concerns and wrap all my imports in one place with a streamline version of this uglification such that it was entirely agnostic of the underlying implementations.

That is, I'd use a module such as https://www.npmjs.com/package/gulp-wrap-umd.

[kirill-konshin]

I don't see any build procedure used in this particular project so far. If there were any -- your suggestion would be viable.

Second, there is ALREADY a hack for NodeJS exports, so why not to put one extra for RequireJS?

[dcousens]

@kirill-konshin as I mentioned, the bower hack is intended to be removed, and replaced with a pre-'compiled' distribution (not ideal, but such is bower).

The node module.exports is because this is an NPM module at its core.

I don't see any build procedure used in this particular project so far. If there were any -- your suggestion would be viable.

This module doesn't need a build process, its a simple module. I'd leave the build process up to the user in their project.

[kirill-konshin]

User may not have build procedure at all. And I doubt that anybody would add build procedure just because one simple module requires it in order to be used ;)

Do not think or make decisions on behalf of your consumers. You never know what setup they have.

The bower hack is intended to be removed, and replaced with a pre-'compiled' distribution
This module doesn't need a build process, its a simple module

I see contradiction here, so does it require build or not? ;)

[kirill-konshin]

I have updated PR, shorter version of RequireJS compatibility export was used.

[dcousens]

I'm still against adding this, however, the shorter version is definitely
more acceptable (and isolated, as a change), therefore I leave the verdict
up to @JedWatson.
On 22 Apr 2015 6:26 pm, "Kirill Konshin" notifications@github.com wrote:

Reopened #30 #30.

—
Reply to this email directly or view it on GitHub
#30 (comment).

[JedWatson]

Good thread, @dcousens and @kirill-konshin - appreciate the thought that went into this.

I'm happy with the updated (shorter) version, it's simple and consistent with the other check so I'll merge this and publish a new version.

Thanks!

[kirill-konshin]

Just FYI.

The original UMD wrapper I've pushed (the one that was discussed) had one extra advantage: classNames.noConflict() mode (when used as global).