TiddlyWiki / TiddlyWiki5

A self-contained JavaScript wiki for the browser, Node.js, AWS Lambda etc.
https://tiddlywiki.com/
Other
7.98k stars 1.18k forks source link

Refactor routes from server.js as separate modules #512

Closed Jermolene closed 4 years ago

Jermolene commented 10 years ago

The route modules need to be ordered, so we should use the same priority field mechanism that the syncer uses for saver modules. The existing implementation of Syncer.prototype.initSavers needs to be abstracted so that the mechanism can be reused.

The guts of a route plugin might look like this:

exports.name = "http-delete-route";

exports.priority = 0;

exports.method = "DELETE";

exports.path = /^\/bags\/default\/tiddlers\/(.+)$/;

exports.handler = function(request,response,state) {
    var title = decodeURIComponent(state.params[0]);
    state.wiki.deleteTiddler(title);
    response.writeHead(204, "OK");
    response.end();
}
pmario commented 10 years ago

One plugin could contain serveral related routes?

Jermolene commented 10 years ago

@pmario yes with this approach, a plugin could contain multiple routes, but each route would need to be a separate module. It probably would be useful to be able to pack multiple routes into each module (implying we'd not be able to reuse the initSavers logic for handling priorities).

natecain commented 10 years ago

I suspect that we will see more use cases for this, like #511, starting to arise.

The way server.js is integrated has been a "pain point" for me, personally, since I made the jump to TW5. Integrating other parts of the NodeJS ecosystem with the TW5 "server side framework" has always been far more complicated and less clean than it should be. I'd like to start work toward remedying this.

@Jermolene: The proposed structure for route handler plugins looks like it would be good. However, I have two thoughts.

First, I'd like to see certain elements like priority and method worked into field structure, instead of module exports, if possible. It seems advantageous to have an architecture where non-developer users could make some configuration changes to their stack without a requirement of knowing any Javascript syntax.

Second, I like that routes are ordered by priority, so if multiple routes "overload" each-other there is a resolution order. However, I feel that the particular signature of the handler function misses a subtle point here. It is possible that two (unrelated & from separate plugins) route handlers would be intended to run over the same route! In particular, the signature should probably align with the nodejs ecosystem convention of "function(request, response, next)" (and/or "function(err, req, res, next)") with "state" either being attached onto the request object (as is common with express/connect stuff) or being mapped to 'this' from the caller side in server.js. With such a signature route handlers could be stacked on overloaded url patterns, all executing in turn, and existing handler middle-wares from npm could be trivially wrapped and "dropped in" as a part of that stack.

I'd also prefer to see a setup where multiple routes could be defined within a single module, as well, but I understand how this could significantly complicate matters of implementation.

Some of these details also may touch on our ongoing Hangouts conversations around concepts like aligning with nobackend/unhosted architectures.

We should discuss this some tomorrow, and then I may begin on an implementation - time permitting.

Jermolene commented 10 years ago

Hi @natecain great, I'm definitely interested in making TW5 be a good citizen in terms of Node.js middleware conventions. It would be nice to produce a demo app based on something like Express that uses TW5 as a middleware component.

First, I'd like to see certain elements like priority and method worked into field structure, instead of module exports, if possible

That makes sense. We can't really use the fields of the plugin containing the route modules because we don't want people having to modify plugin tiddlers in order to configure them. The problem with using fields on the individual modules containing routes is that we don't currently retain any information about the fields on module tiddlers.

Maybe we could make the server configuration reside within tiddlers more akin to the way that we handle the sidebar tabs in the browser: using tags to select and order routes. The tiddlers that would be carrying the tags would configure each route.

However, I feel that the particular signature of the handler function misses a subtle point here. It is possible that two (unrelated & from separate plugins) route handlers would be intended to run over the same route!

OK, that makes me think that maybe we shouldn't add a priority system for routes, but instead rely on the existing priority field handling for plugin tiddlers. That means that people could (at a pinch) adjust the priority field of a plugin to adjust the processing order between plugins.

In particular, the signature should probably align with the nodejs ecosystem convention of "function(request, response, next)" (and/or "function(err, req, res, next)") with "state" either being attached onto the request object (as is common with express/connect stuff) or being mapped to 'this' from the caller side in server.js

That sounds good.

Jermolene commented 4 years ago

This was implemented in v5.1.18; see https://github.com/Jermolene/TiddlyWiki5/tree/master/core/modules/server/routes