Simplify middleware used in tests to parse the body #459
Conversation
cibernox
force-pushed
the
simplify-post-middleware-used-in-testing
branch
from
5c63e9a
to
f906581
Compare
| } else { | ||
| next(); | ||
| } | ||
| fastbootMiddleware(req, resp, next); |
There was a problem hiding this comment.
Shouldn't this middleware only be called when it is a POST request and when it is a base page request? if you have tests that request for assets, this would result in an error.
There was a problem hiding this comment.
I think we don't need to check for the method. POST, PUT and PATCH all can have body. GET will not, but it doesn't harm to leave it, right?
About the "base page request" what do you mean? That the request has a header requesting for HTML?
There was a problem hiding this comment.
GET will not, but it doesn't harm to leave it, right?
yes
About the "base page request" what do you mean? That the request has a header requesting for HTML?
I meant fastboot is serving the index.html page to you with the rendered DOM in it. You only want to send the request to the fastboot-express-middleware if it is a index.html request. You know it is because ember-cli exposes it via req.serveUrl being defined.
There was a problem hiding this comment.
You only want to send the request to the fastboot-express-middleware if it is a index.html
Will that still work if the user is requesting /some/nested/url?
There was a problem hiding this comment.
yes if you are passing the accept/html request header.
There was a problem hiding this comment.
@kratiahuja I tried but I can't check for req.serverUrl yet until we fix this line in ember-cli
There was a problem hiding this comment.
I'll work on a PR to ember-cli itself right now.
In preparation of the docs, I've simplified it so it's more idiomatic.
cibernox
force-pushed
the
simplify-post-middleware-used-in-testing
branch
from
f906581
to
d3e144b
Compare
|
@cibernox do you want me to wait on this PR or release? |
|
Release if you can. This is just a simplification on the tests. Purely for my own satisfaction. |
|
@cibernox 1.0.2 is released. |
In preparation of the docs, I've simplified it so it's more idiomatic.