-
Notifications
You must be signed in to change notification settings - Fork 38
Is this over-designed? #72
Comments
As to the question of being over-engineered... ┐(ツ)┌ ... that's a pretty subjective question we shouldn't, and won't, attempt to "resolve" To attempt to get at an actual issue here, are you concerned about route order? Do you have a use case to consider? |
@magicdawn I believe you may be confused by our implementation. Routes register through The sub app you see is popped off the stack immediately after it is mounted. The sole reasons for using this pattern (which I call the "ephemeral app pattern") are 1) to make What we actually do is create a Now there are many benefits to using a Router instance instead of registering routes directly against the parent. For one, you can namespace routes by setting a mount path for the If none of those grab you, perhaps the performance benefit will? Registering a hundred routes against the parent app means that your route resolution could potentially have to do 100 tests to resolve the route (because it will be a flat list of routes). If, however, you can register your routes under a namespace, you can turn your flat route map into a tree structure. Now your flat, 1-level route map will be a more tree-like 2 (or more) level route map. I'm not sure what you're asking with your route order comment—could you please clarify? |
If slightly more deterministic route ordering is what he'd like, I've started that in PR #47. |
I don't see any point on creating a sub app to take over all traffics.
And this company with plenty of problems. Such as route orders.
https://github.com/krakenjs/express-enrouten/blob/v1.2.x-release/index.js#L120
https://github.com/krakenjs/express-enrouten/blob/v1.2.x-release/lib%2Fregistry.js#L65
The text was updated successfully, but these errors were encountered: