Document new FetchState methods - #14476
Conversation
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
Preview deployment ✅ Deployment complete!
|
ArmandPhilippot
left a comment
There was a problem hiding this comment.
Thanks, Matthew! I left a few nits, but otherwise this looks good to me. I'm only confused with state.finalize() (see my comment).
| <Since v="7.3.0" /> | ||
| </p> | ||
|
|
||
| Applies the response finalizers registered with [`state.addResponseFinalizer()`](#stateaddresponsefinalizer) and returns the resulting response. Call this after your last response handler when composing a pipeline from individual handlers instead of `astro()`: |
There was a problem hiding this comment.
The description seems okay to me, but then I'm confused looking at the code snippet: you say "Applies the response finalizers registered with state.addResponseFinalizer()" but the code snippet doesn't show that.
So I wonder: does state.finalize() work without addResponseFinalizer()? Or should the code snippet include addResponseFinalizer() to better illustrate the use of finalize()?
There was a problem hiding this comment.
It just returns the same response if you don't have any finalizers. But the idea is that anything that has the FetchState can add a finalizer, so you want to run this even if you didn't add one yourself. I can add an one to the example of that helps to clarify though.
There was a problem hiding this comment.
Ah, thanks for the explanation!
If finalize() is useless without addResponseFinalizer() (ie. same response), I think the description is fine (no need to say they can call it, but it does nothing...). But, yes, I think it might be helpful for users to see a "real" use case with addResponseFinalizer().
Co-authored-by: Armand Philippot <git@armand.philippot.eu>
Co-authored-by: Armand Philippot <git@armand.philippot.eu>
ArmandPhilippot
left a comment
There was a problem hiding this comment.
So, apart from my last nit and the code snippet which I think could be improved to help users (still a nit that said: better to have one than nothing at all), everything LGTM! Thanks, again.
Co-authored-by: Armand Philippot <git@armand.philippot.eu>
Description (required)
Documents new FetchState methods added in withastro/astro#17708
References
withastro/astro#17708