Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The parameters doesn't work #64

Closed
dancespiele opened this issue Feb 18, 2018 · 12 comments
Closed

The parameters doesn't work #64

dancespiele opened this issue Feb 18, 2018 · 12 comments

Comments

@dancespiele
Copy link

The parameters doesn't works. This I have in routes

SimpleRouter {
  routes: 
   [ { path: '/user/:id', method: 'GET', fn: [Function: getUser] },
     { path: '/', method: 'GET', fn: [Function] } ] }

If I request with curl -X GET http://localhost:3000/user/4 it return undefined but I I change the router without parameter and request curl -X GET http://localhost:3000/user it receive a body correctly

@Dashron
Copy link
Owner

Dashron commented Feb 18, 2018

Colon isn't an accepted URI templates prefix in this router. Use # for numbers and $ for anything else.

Your example should look like /user/#id.

I'm a little behind on the docs for this, I'll give it a pass tomorrow.

@Dashron
Copy link
Owner

Dashron commented Feb 18, 2018

I just gave a first pass to the SimpleRouter docs: https://github.com/Dashron/roads#simplerouterroad-road. Let me know if you have any questions.

Also, I wanted to give you heads up that I'm moving the body parsing into middleware. I'll make sure to document the change.

@dancespiele
Copy link
Author

dancespiele commented Feb 18, 2018

  • @Dashron I like the idea to put the prefix # and $ because everything that you have to expecify the type is good. I think in the docs is mixing ‘SimpleRouter.midleware(args...)‘

@Dashron
Copy link
Owner

Dashron commented Feb 18, 2018

At the moment I'm keeping that private, I should probably rename it to _middleware. There's some weird stuff with the request context that makes it complex to open up publicly.

You should use applyMiddleware.

@dancespiele
Copy link
Author

Ok, but I have a question. If I want to create a middleware before or after of execute a route, how would I do it? Image that I want to check something before of execute this route, a validation data for example.

@Dashron
Copy link
Owner

Dashron commented Feb 19, 2018

There are some examples in the use documentation here: https://github.com/Dashron/roads#use-callback. The tl;dr is that you should add middleware before you mount the router for any before-route action, and you have two options for post-route actions.

  1. From within middleware added before the route action, put your "after-route" logic in the "then" handler of the promise returned from next();
  2. Add middleware after the route action and make sure your route calls next();

Let me know if you want further examples

@dancespiele
Copy link
Author

It would be cool an example about this case that I mentioned before please

@Dashron
Copy link
Owner

Dashron commented Feb 23, 2018

sure. I just added a new section to https://github.com/Dashron/roads/blob/master/README.md#use-callback called "How do I control the order my logic executes?". Let me know if that clears it up

@dancespiele
Copy link
Author

dancespiele commented Feb 24, 2018

Yes now I understand completly. For example to create a before and after middleware I can do this.

app.use(function (method, path, body, headers, next) {
    console.log('A ' + method + ' request was made to ' + JSON.stringify(path));
    if(next) return next();
});

router.addRoute("GET", "/user/#id", (url: any, body: any, headers: Headers, next: Function) => {
    console.log(`These are the parameter user ${JSON.stringify(url.args)}`);
    return next();
});

router.addRoute("GET", "/person", (url: any, body: any, headers: Headers, next: Function) => {
    console.log("This is a example");
    return next();
});

router.applyMiddleware(app);

app.use((method, path: any, body, headers, next) => {
    if(path.path === '/user/5') {
        console.log("10");
        return new Response({id: "10"}, 200);
    }
    if(next) return next();
});

@Dashron
Copy link
Owner

Dashron commented Feb 24, 2018

I have only two small comments on your example.

  1. You will always have next, so you shouldn't need to wrap that in an if statement. Even if there is no additional middleware you will get a next that just immediately returns.
  2. The final function (with the path check) might not always execute. If your logic in the router doesn't call next(), it won't check for /user/5. To ensure you don't have to remember to call next() in every single router function I recommend you include that after logic in your "this is an example" section like this.
router.addRoute("GET", "/person", (url: any, body: any, headers: Headers, next: Function) => {
    console.log("This is a example");
    return next()
    .then(function (response) {
        if(path.path === '/user/5') {
            console.log("10");
            return new Response({id: "10"}, 200);
        }
    });
});
  1. If you provide arrow functions to the use function, they won't have the proper request context. This is a limitation of arrow functions and unfortunately something I'm not able to work around.

@dancespiele
Copy link
Author

dancespiele commented Feb 25, 2018

About the if was some problem with the typescript that I put it as optional argument and typescript force you to put if, anyway I changed it and now It is required to put it as argument then is no necessary if.
About the second point I have to do like this how I'm doing, I know that I have to call the next function If I want execute the next middleware otherwise I cannot create decorations https://github.com/spieljs/spiel-server/blob/master/test/server.ts, https://github.com/spieljs/spiel-server/blob/master/src/server/set-router.ts

@dancespiele
Copy link
Author

I consider that the parameters is resolved because of this I closed this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants