Update README #25
Update README #25
Conversation
|
This has been updated heavily to reflect recent decisions regarding naming and scope. Any feedback would be greatly appreciated. cc @http-interop/http-middleware-contributors |
shadowhand
force-pushed
the
fix/import-meta-as-readme
branch
from
9c33a42 to
1db1088
Compare
shadowhand
changed the base branch from
master
to
feature/import-meta-as-readme
| The general concept of reusable middleware was popularized by [StackPHP][stackphp]. | ||
| Since the release of the HTTP Messages standard, a number of frameworks have | ||
| adopted middleware that uses HTTP Message interfaces. | ||
| The HTTP Messages specification contain any reference to HTTP Middleware. |
| adopted middleware that uses HTTP Message interfaces. | ||
| The HTTP Messages specification contain any reference to HTTP Middleware. | ||
|
|
||
| The design pattern used by middleware has existed for many years as [pipeline][pipeline], |
There was a problem hiding this comment.
s/[pipeline][pipeline]/[the pipeline pattern][pipeline]/
| 3. A `callable` that receives the request to dispatch next middleware. | ||
|
|
||
| 1. An object that represents an HTTP request. | ||
| 2. A delegate that receives the request to dispatch next middleware. |
There was a problem hiding this comment.
s/dispatch next middleware/dispatch the next middleware in the pipeline/.
|
|
||
| ``` | ||
| function handle(Request $request, callable $next): Response | ||
| handle(Request $request, $type, $catch): Response |
There was a problem hiding this comment.
This contradicts line 119, where you indicated that the StackPHP "style" is fn(request, frame): response. I'm thinking that the narrative at line 119 should likely not mention Stack at all.
There was a problem hiding this comment.
Stack implements has multiple parameters, but none of them are $response. As such, I think it still constitutes a widely used example of single-pass.
| server request interface, external requests are typically handled asynchronously | ||
| and would need to return a promise of a response. This is outside the scope of | ||
| this proposal and would complicate matters for any future proposal that is focused | ||
| on client side request processing. |
There was a problem hiding this comment.
I think this paragraph needs some expansion:
- You gloss over "external requests". Maybe expand to say, "For client-side middleware that makes a request to an external resource, you will typically want to react asychronously...", and then detail why async is typical in this situation.
- Why is a "promise of a response" required under async? (I know, but not everyone is familiar with async processing.)
- What is a "promise"? (I know, but will every reader coming to the spec? Link to the Promises A+ spec
|
What I mean by my approval here is, "approved with minor comments", just the minor corrections suggested by @weierophinney. |
Rework document based on current interfaces. Add more detail regarding design decisions and clarify intent to focus on server middleware.
shadowhand
force-pushed
the
fix/import-meta-as-readme
branch
from
1db1088 to
d33ce2d
Compare
Rework document based on current interfaces. Add more detail regarding
design decisions and clarify intent to focus on server middleware.