solidusjs / solidus

A simple server that generates pages from JSON and Templates
MIT License
28 stars 7 forks source link

Dynamic redirections. #109

Closed joanniclaborde closed 9 years ago

joanniclaborde commented 10 years ago

Fixes #108

The redirects.json file is renamed into redirects.js, and it is now a module which must export the array of redirections. Redirections can have dynamic parts, or be regular expressions. Self-explanatory example:

module.exports = [
{
  from: "/redirect1/{dynamic}/{route}",
  to: "/new/{route}/{dynamic}"
}, {
  from: /\/redirect2\/(\d+)-\d+-(\d+)-(\d+)/,
  to: "/new/{1}/{0}/{2}"
}];
jameslovejoy commented 10 years ago

Neat :+1:

pushred commented 10 years ago

Rebased, we're going to give this a go on a Jason Aldean splash page that's going to live at www.jasonaldean.com. Need to redirect all the original Clique Tools routes to some temporary subdomain till we go live with the rebuild.

pushred commented 9 years ago

We didn't end up pushing that live, but regex worked out great. We should merge this for 0.3.0.

joanniclaborde commented 9 years ago

This is a breaking change, it needs to go in 1.0.0.

joanniclaborde commented 9 years ago

Rebased, delete your local branch!

joanniclaborde commented 9 years ago

Note: this is merged into https://github.com/solidusjs/solidus/tree/1.0.0

pushred commented 9 years ago

Looks like I can't match things in a query string, is that by design since routing is otherwise path-based? In my case I'm trying to merge a query string from a redirect with another at the target URL.

joanniclaborde commented 9 years ago

http://expressjs.com/3x/api.html#app.VERB

Query strings are not considered when peforming these matches, for example "GET /" would match the following route, as would "GET /?name=tobi".

It looks like the Express router is too basic for your needs :(

pushred commented 9 years ago

What about actually executing some code to get the value of to? In my case I've got a series of routes that spell out the names of months, i.e. january, october. But in the destination I need them to be their numbered equivalents. to could accept a function with any variables or capture groups as arguments that returns the destination path. That would provide a ton of flexibility in situations like this.

joanniclaborde commented 9 years ago

Something like this (I'm about to commit)? The dev can do his own interpolation with params, or let Solidus expand the string returned by the function. Or do both:

{
  from: "/redirect8/{dynamic}/{route}",
  to: function(params) {
    return "/new/{route}/" + params.dynamic.toUpperCase();
  }
}
pushred commented 9 years ago

Perfect!

joanniclaborde commented 9 years ago

Fixed, tell me if you need it in the 108-dynamic-redirects-longer-timeout or 1.0.0 branch.

pushred commented 9 years ago

108-dynamic-redirects-longer-timeout would be great, thanks

joanniclaborde commented 9 years ago

Done.

pushred commented 9 years ago

Works great! What are your thoughts on resource fetching in a redirect? I've got a scenario where a content item no longer has a dedicated page, but rather appears in a parent page as an anchor link. Problem is in order to get the necessary data for routing to the parent I need to make a request.

This would likely also solve any scenarios in Solidus across the board where a given resource's parameters similarly requires a dynamic value from a sibling resource. This is often due to constraints in an API that are sometimes addressed with field expansion. This has been avoided in the past with XHR hacks and in the worst cases, meta redirects. These target functions seem like a much better place for this code. They've already taken us beyond simple redirects and effectively provided a place for more complex routing rules than the filesystem allows.

pushred commented 9 years ago

Also currently I need to restart the server to pickup any changes I make to redirects. Can this be added to the file watcher, refreshing the logic whenever the file is changed?

pushred commented 9 years ago

Anyway to support named functions with additional arguments? Doesn't seem to work.. no error handling either, the redirects just break. Was trying to DRY up some logic, like:

function normalize( params, targetPath ){

  // do stuff to params.param
  ...

  return targetPath + result
}

module.exports = {
  'from': '/path/{param}',
  'to': normalize(params, '/new/')
}, {
  'from': '/path2/{param}',
  'to': normalize(params, '/new2/elsewhere/')
}

For now I'm just calling my function from within the anonymous callback.

joanniclaborde commented 9 years ago

Your normalize function needs to return a function, that will get executed at runtime when the redirection happens.

joanniclaborde commented 9 years ago

Re: resource fetching before redirecting.

Doesn't #125 address this problem?

joanniclaborde commented 9 years ago

Re: reloading redirects.js

The file is already watched, but the existing redirects were not removed before the file was reloaded. https://github.com/solidusjs/solidus/commit/4e46807741d3aaf5da05e42dce3172628472ef44 fixes this.

pushred commented 9 years ago

You mean like this?

return function(){  
  return path
    ? path + normalized
    : normalized;
  }
}

Doesn't seem to get called.

pushred commented 9 years ago

Oops I was on the wrong route!

pushred commented 9 years ago

Same issue though.

joanniclaborde commented 9 years ago
function normalize( params, targetPath ){

  // do stuff to params.param
  ...

  return targetPath + result
}

module.exports = {
  'from': '/path/{param}',
  'to': function(params) {return normalize(params, '/new/')}
}, {
  'from': '/path2/{param}',
  'to': function(params) {return normalize(params, '/new2/elsewhere/')}
}
pushred commented 9 years ago

Yeah that's what I'm doing now, was trying to avoid that =)

pushred commented 9 years ago

sooo.. Let's merge this and bump to 1.0 :+1:

pushred commented 9 years ago

Also https://github.com/solidusjs/solidus/tree/108-dynamic-redirects-longer-timeout