window.tinycolor is not defined if JS loader is used

[olee]

When a loader like SystemJS or RequireJS is found, tinycolor skips registering the window.tinycolor variable. This breaks other modules that don't use a loader and depend on window.tinycolor.

[bgrins]

I thought it was using a pretty standard setup for module registration. Do you have an example where this doesn't happen?

[olee]

I was trying to use https://github.com/brianpkelley/md-color-picker which needs tinycolor with typescript.
md-color-picker relies on tinycolor being defined in window, which was missing because SystemJS was active.

[bgrins]

I guess I'm not sure of how to distinguish between 'a module loader is present' and 'this script is being loaded with a module loader'. I'm not sure I've seen UMD use both define or module.exports and also setting to 'window'

[olee]

Just remove the else and always add tinycolor to window.
It should not skip that step even if a loader is present.
JQuery for example does it the same way.

[bgrins]

Just remove the else and always add tinycolor to window.

But what about in the node js environment?

[olee]

Check if window is defined? 😏
I don't see any problem with that.
At least I don't see any other good solution to the problem which isn't a hack....

[bgrins]

I imagine there's a case where someone is using a module loader because they don't want libraries to pollute the global namespace. I think what it's doing now is the 'standard' way to register modules with UMD. Are you able to load tinycolor via SystemJS and then set window.tinycolor after that finishes loading?

[jt3k]

yes
if u need link to tinycolor in property of window object then:

require(['tinycolor'], function(tinycolor){
  window.tonycolor = tonycolor;
});

this is the right way

[olee]

Well - I solved in kinda that way for now, too.

I imagine there's a case where someone is using a module loader because they don't want libraries to pollute the global namespace.

Yes, but not quite. It's more to prevent collisions than just populating the window namespace.
If a loader is used like it should be, a window namespace with many properties will not cause any problems, but can still help to solve backwards compatibility issues with libraries that don't support module loading yet. That's why I suggested this.

TL;DR: Can you find any negative effect of registering tinycolor in the window namespace?
If you can think of one then of course, it would be better to leave it as this and do by with other solutions like the one mentioned, but I at least can't think of one.

[jt3k]

@olee Imagine you have legacy code which uses a global variable with the name tinycolor...

Way with require more flexible.

[bgrins]

Agreed with @jt3k and #122 (comment) is a workable solution to the problem. Closing this out.