express-enrouten icon indicating copy to clipboard operation
express-enrouten copied to clipboard

Add a way to pass configuration into handler setup controllers

Open aredridel opened this issue 10 years ago • 22 comments

As a vague example, maybe something like accepting functions in the form of

module.exports = function (router, config) {
}

This would solve one of the most common questions I get in the context of Kraken: "How do I handle configuration? I can't get to it from here"

aredridel avatar Dec 02 '14 19:12 aredridel

+1 (I'd use this feature today)

sixlettervariables avatar Dec 02 '14 19:12 sixlettervariables

So the complexity here is that enrouten has no concept of config. Thinking this over, I think the simplest way to implement this would be to add another internal shortstop handler to confit—similar to the config handler—that would be replaced with a reference to the completely built config factory. That way, it could be passed as an argument (or option) to enrouten.

Just a thought.

jasisk avatar Dec 02 '14 19:12 jasisk

Yeah. Some sort of opaque thing passed in? If it's config, yay! when used by Kraken.

aredridel avatar Dec 02 '14 19:12 aredridel

Yep, this is a long time coming but really is a form of DI. As I start contemplating I welcome any thoughts or ideas.

EDIT: And yes, I'll start digging into your suggestion @jasisk.

totherik avatar Dec 02 '14 21:12 totherik

One thing that just occurred to me is that this will necessarily force a major version change (I was hoping it wouldn't). The current rule for directory mode is that the traversal mechanism will only initialize files that accept a single argument, allowing route definitions to be intermingled with other files. By having user-defined arguments the mechanism for identifying those files becomes much more murky.

totherik avatar Dec 03 '14 16:12 totherik

Oh, that is just unfortunate!

aredridel avatar Dec 03 '14 17:12 aredridel

It may be a blessing in disguise. Gives us a little more latitude to take advantage of better directory layout and improve config. I've put together a very rough proposal if anyone has feedback.

totherik avatar Dec 03 '14 17:12 totherik

Aye. That seems like an awful lot of DI machinery and jargon to drag in, and it means that we're coupled to doing dependency injection our way, which just makes it unpleasant for people who have another opinion there.

aredridel avatar Dec 08 '14 17:12 aredridel

I see the intent, but something is rubbing me really wrong about it.

aredridel avatar Dec 08 '14 17:12 aredridel

Yep, re: DI. I was considering using an existing implementation by default and injecting the router itself that way, but I haven't yet found a "lightweight" module, hence the code I put together. If we go through with this I'd prefer to defer to an existing module should we go in whole hog, and use it to manage all dependencies, including router.

totherik avatar Dec 08 '14 17:12 totherik

I dunno. It feels a lot like a lot of conceptual overhead to use an otherwise simple module.

aredridel avatar Dec 08 '14 17:12 aredridel

I'm im not sure that simplicity and configurability are mutually exclusive. The request to inject arbitrary objects during initialization isn't the basic, simple use-case here. A smart default of always injecting the router will most certainly help.

totherik avatar Dec 08 '14 18:12 totherik

Indeed.

aredridel avatar Dec 08 '14 18:12 aredridel

regarding the single argument function, just because a function signature has a single argument doesn't mean the function cannot be called with additional arguments

grawk avatar Dec 11 '14 16:12 grawk

True, but I'm personally not willing to codifyvar myvar = arguments[1]; as a desired pattern. :)

totherik avatar Dec 11 '14 16:12 totherik

What about attaching the config as a property of the router?

grawk avatar Dec 11 '14 16:12 grawk

That's interesting. We already provide a facade there.

jasisk avatar Dec 11 '14 16:12 jasisk

Well attaching config explicitly so will create a tight coupling between kraken-js and enrouten, something that I'm trying to avoid. The only way to avoid that would be to support something like an additionalProperties config property that would be an object of arbitrary stuff which would get attached to the router instance.

With a design that supports multiple arguments the default case (just router) could possibly keep the current scanning mechanism, but when additional dependencies are configured to be injected it could switch into a more liberal scanning mode. That would preserve backward compat.

totherik avatar Dec 11 '14 16:12 totherik

the single argument could be an object but the user would need to be aware of when they have triggered DI in order to pull the router argument apart

grawk avatar Dec 11 '14 16:12 grawk

+1 to adding additional params as a config

pvenkatakrishnan avatar Dec 11 '14 16:12 pvenkatakrishnan

Does someone want to put together a proposal?

totherik avatar Dec 11 '14 19:12 totherik

I don't have much time today and tomorrow. But if it's still dangling when I'm free I would like to take a crack at it.

grawk avatar Dec 11 '14 20:12 grawk