search

mozilla / connect-cachify

Express connect middleware to provide easy frontend caching for Node.js
Mozilla Public License 2.0
132 stars 21 forks source link

design question: global variables #11

Closed lloyd closed 11 years ago

lloyd commented 12 years ago

because connect cachify uses global variables, it's not possible to have two instances.

Is this really a problem? Would there ever be the case where someone wanted to apply cachify in a large project and serve two sets of resources with different prefixes (perhaps from different CDNs?). Back in my yahoo days we had 4 different cdns for different types of resources, with different costs and latency characteristics, and we did indeed serve them from the same site, but this all might be too fringe to worry about.

Whatever your answer, perhaps cachify can be improved regardless:

if you do not care to support two different cachify "instances" in the same process with distinct configurations, maybe you could defend against this in .setup() and throw an exception if someone tries to invoke setup twice. Maybe you don't need to have the middleware returned from .setup() (based on my knowledge of node convention, this implied to me that there were no globals). And maybe you simply hook the middleware on exports.middleware.

I was confused at first with cachify because I didn't understand how the cachify_js call could get the context passed into setup. If setup and middleware were distinct, I might have realized the design earlier and not have been confused.

If you think cachify should not use globals and it should be possible to have multiple "cachify instances" in one process, then maybe the user allocates a cachify instance with new Cachify([options]), and that instance has .middleware, .hashify(), etc, on it.

Even if you don't care about the feature (and again, it's fringe), I think returning an object might be more idiomatic and intuitive. BUT, that's all subjective anyway!

ozten commented 12 years ago

This is awesome feedback and I'm new to writing idiomatic connect/node modules.

Noodling!

ozten commented 11 years ago

I haven't gotten around to this, but I'd still like to remove global variables.

seanmonstar commented 11 years ago

I believe all the global variables were removed with #19, right?

ozten commented 11 years ago

Oh, good call @seanomstar. Closing. We can make new pull requests or issues for further evolution of the code.