Include an optional restRoute config

[ChromeQ]

So that I may override in my own configs to point to my own main route. /public directory is often used as a grunt build directory so a possibly dev config route is useful.

[zemirco]

Where do you put your index.html if not in public/ ?

It's probably a bit more complicated than your PR because you could also use a .jade view as the starting point for your SPA.

[ChromeQ]

I built the app using yeoman and the main front end files live under /web. The grunt build task creates the /public folder for production ready state. Sending the routes to /public/index.html isn't much good for my app or anyone using yeoman I should think. And by sending the get route of /signup/:token to index.html means it misses out getting to the getSignupToken method in lockit-signup which validates the users email in the DB.

I am trying to find a solution at the moment and trying to understand all the routes in and out of each of the modules.

The PR I sent was a quick fix to use the config path that I could override in my own config and defaulting to your /public/index.html that you already hard coded as the res.sendfile()

[zemirco]

And by sending the get route of /signup/:token to index.html means it misses out getting to the getSignupToken method in lockit-signup which validates the users email in the DB.

Here is how it works.

A user gets an email with a link pointing to /signup/:token. When visiting this route the server sends index.html and your client router takes over. Your app controller makes an AJAX request to /rest/signup/:token and gets 200 if the token is correct. Then you should show a message that the user is now authenticated and can log in with the credentials.

See the AngularJS example router examples/angular/public/js/app.js#L24 and controller examples/angular/public/js/controllers.js#L69.

[ChromeQ]

I did think that was a way to do it but it seems silly to load the front end app to just make another http request when the original could hit the route directly via the email link. That was my thinking for the first pull request when I deleted the lines that added /rest to the routes in lockit-signup. Now with the routes in the main lockit module catching it, I have played with doing a res.redirect to /rest/signup/:token instead and that seems a bit more direct (for this and the other routes as config.rest is a global config setting), it means it will hand off the route handling to the module concerned, although I have had to put handleResponse: false in my own config so my express routes can respond.

Perhaps that your suggestion is the slightly cleaner and more modular within the lockit ecosystem and letting the responsibility of the developer to know what I'm doing by setting rest: true in my configs.

In that case would it be a good idea to let the config dictate the location of the index.html so that in my case using yeoman that makes /web the (dev)public and not the normal /public folder, this way it offers a bit of flexibility to all users. I could edit my grunt file to update this route dependent on environment. Of course the default in the config.default.js can be /public/index.html as per my PR.

The issue and complication still remains of course if the index.html doesn't exist and should be rendered via jade or similar (handlebars in my case). But I think that is a bit more involved at the moment and requires more work than a simple config addition.

Hope this helps and thanks for the clarification.

[zemirco]

Thanks a lot for all the detailed comments!

Mixing server-side routing and client-side routing is not a good idea. It will get very confusing.

I'll implement a solution where everybody is able to set the starting point of their SPA manually without too much hassle.

[ChromeQ]

Sorry for the long reply! But I just had another thought, the problem I have at the moment is that you use res.sendfile and if that was changed to res.render then I could point to my handlebars or jade template as express will abstract the rendering and the layout stuff. With sendfile it gets sent as plain html markup, with render it would benefit from the express engine.

[zemirco]

I'll implement a solution where everybody is able to set the starting point of their SPA manually without too much hassle.

That's what I tried to answer with my comment above. Soon you'll be able to send an HTML file via res.sendfile or render any template via res.render.

[ChromeQ]

eek, i think my patch is patching itself! Yes, thanks, res.render would be awesome with a config option.

[ChromeQ]

Hi Mirco, How is it going to implement a solution that allows users to render a starting page when rest=true, interested to see how you implement the res.render and if it could use the engine defined in express rather than using jade.

[zemirco]

Soon, and with your other fixes I'll release a new version. Thanks for all your help!

[ChromeQ]

Thanks for the update. I really think it an improvement but can I ask that you could let me specify a file extension or just use the entire contents of config.restIndexPage, I don't use jade and if we pass the express app to the lock it instance it could be possible to get the render engine property. The issue with this though is that everyone would need to specify the app.engine before initialising lockit. It is very close as it is but possibly checking another config option of the restIndexExtension could be useful for the regex test or if config.restIndexPage is undefined then default should use public/index.html as it did.

Sorry to be a pain, its because I really like the whole idea of lockit and flexibility is often a good thing to developers so I hope this isn't too annoying to hear my opinions all the time! Thanks again, Dav On 3 Apr 2014 19:02, "Mirco Zeiss" notifications@github.com wrote:

Closed #6 https://github.com/zeMirco/lockit/pull/6 via 86ea864https://github.com/zeMirco/lockit/commit/86ea8646469b3ee63addb006d8e3d2047679a0de .

Reply to this email directly or view it on GitHubhttps://github.com/zeMirco/lockit/pull/6 .

[zemirco]

hah I'm so used to Jade that I forgot about other template engines. Thanks for the hint!

How about this?

// anything that doesn't end with .html should be render()ed
if (!/.+\.html$/.test(config.restIndexPage)) return res.render(config.restIndexPage);

// for .html files use sendfile()
res.sendfile(path.join(__parentDir, config.restIndexPage));

[ChromeQ]

I like it!

Thanks so much. On 3 Apr 2014 21:17, "Mirco Zeiss" notifications@github.com wrote:

hah I'm so used to Jade that I forgot about other template engines. Thanks for the hint!

How about this?

// anything that doesn't end with .html should be render()edif (!/.+.html$/.test(config.restIndexPage)) return res.render(config.restIndexPage); // for .html files use sendfile()res.sendfile(path.join(__parentDir, config.restIndexPage));

Reply to this email directly or view it on GitHubhttps://github.com/zeMirco/lockit/pull/6#issuecomment-39499420 .

[zemirco]

Thought about the issue a bit more.

The problem is that a user could actually have .html files (with ejs template engine for example) that should be rendered. So I cannot assume that every .html is used with sendfile().

Getting all the required info just from config.restIndexPage is impossible. I'll have to add another key to the config. Maybe wrap all rest related stuff into one object, like so

exports.rest = {

  // render views or send JSON for single page apps
  enabled: false,

  // set starting page for single page app
  index: 'public/index.html',

  // use view engine (render()) or send static file (sendfile())
  useViewEngine: false

}

That's probably the most flexible solution.

[ChromeQ]

That's a good point that I also didn't think about. I use handlebars usually. But that solution does seem to be the best fit with the extra config key.

Talking of separating some options into another object, I had to make some amends to the couchdb adaptor as I needed to pass some request_defaults such as http proxy, it mostly meant taking the dB name out of the URL and using nano.use(dbname) but I will try to simplify my changes.

Thanks. On 4 Apr 2014 08:11, "Mirco Zeiss" notifications@github.com wrote:

Thought about the issue a bit more.

The problem is that a user could actually have .html files (with ejs template engine for example) that should be rendered. So I cannot assume that every .html is used with sendfile().

Getting all the required info just from config.restIndexPage is impossible. I'll have to add another key to the config. Maybe wrap all rest related stuff into one object, like so

exports.rest = {

// render views or send JSON for single page apps enabled: false,

// set starting page for single page app index: 'public/index.html',

// use view engine (render()) or send static file (sendfile()) useViewEngine: false }

That's probably the most flexible solution.

Reply to this email directly or view it on GitHubhttps://github.com/zeMirco/lockit/pull/6#issuecomment-39537482 .

[zemirco]

Check out the just released version 0.8.0. It should solve both problems. Here is the changelog.

[Micha4711]

I'd like to reopen this issue and make a proposal.

I'm using lockit in a SPA using Ember.js. Routing in Ember is done in the hash part of the URL. So, I want to forward "/signup/a-token" to my frontend with an URL like "index.html#/signup/a-token". Forwarding via sendfile() does not feel suitable for my use case. For the SPA case, I'd like to have a custom hook, where I can forward to client routes in a non restricted, flexible way.

I would prefer something like this for the registered routes:

      router.get(route, function(req, res) {
        // check if user would like to render a file or use static html
        if (config.rest.useViewEngine) {
          res.render(config.rest.index, {
            basedir: req.app.get('views')
          });
        }
// PROPOSAL START
        else if (config.rest.customRouter)
        {
          config.rest.customRouter({
            "req": req,
            "res": res,
            "route": route
          });
        }
// PROPOSAL END
        else {
          res.sendfile(path.join(__parentDir, config.rest.index));
        }
      });

Then I would have full control over all routing to the client by catching this hook:

exports.rest = {
   customRouter: function(p)
   {
      if (p.route.match(/^\/signup/))
      {
         p.res.redirect('/index.html#/signup/' + p.req.params.token);
      }
   },
   useViewEngine: false
};

I think this would be more flexible and suitable for different use cases in different client frameworks.

What do you think? Or am I misunderstanding the way of forwarding requests to the client?

[zemirco]

Hash based routing was hack to make SPAs work in the past. Nowadays modern browsers support the history api. Edit your Ember app and set location to 'history'.

App.Router.reopen({
  location: 'history'
});

This removes the hash from the URL and everything should work as expected.