OverZealous / cdnizer

Node module for replacing local links with CDN links, includes fallbacks and customization
MIT License
52 stars 24 forks source link

cdnizer export function opts #23

Closed backsapce closed 8 years ago

backsapce commented 8 years ago

the cdnizer object init opts when its born; but in some situation.the option may different. so i think the main export function can add optional argument named opts. when call the function.if opts coming in. the session opts can extend global opts. if you agree with me.i can cook it :)

OverZealous commented 8 years ago

It seems like a really limited need. I'm just not sure what options would be overridden in this manner.

The problem I have with it is the options doesn't really contain the sort of options that make sense to override right before you run the function, rather than at creation time.

Personally, I think it makes a lot more sense to just make multiple cdnizer instances. This makes the intention clearer in how it's being used.

tomByrer commented 8 years ago

I can see a (limited) use case; A/B testing of CDNs. (At jsDelivr, we get complaints of other Google CDNs & occasionally CDNJS being slower in China.) Though you're right OZ; could be run multiple times. Perhaps he's using Gulp/Grunt or other build tool? Maybe more clear usage is better?

OverZealous commented 8 years ago

That doesn't really make any sense, though. If you are using my gulp plugin, you have to pass in a unique set of options when configuring the stream, so this functionality won't help. (I don't know if there's a grunt plugin or how that is configured.)

I don't really see how changing the options when calling cdnizer(file) would help with A/B testing. You might as well do require('cdnizer')(opts)(file) every time if you are going to change the options on every file.

I don't think the usage instructions are unclear. I mean, I call the object returned from require('cdnizer') a cdnizerFactory. This is a pretty common way for node libraries to work.

tomByrer commented 8 years ago

Good points, ty.

OverZealous commented 8 years ago

I've thought about this further, and I believe this unnecessarily complicates the library, since it's easily managed outside the library by creating a new cdnizer if you need different options.