Added getHeader and setHeader to the preload context#1517
Added getHeader and setHeader to the preload context#1517ehrencrona wants to merge 2 commits intosveltejs:masterfrom
Conversation
|
What do |
|
@Conduitry Oh, I was under the impression it's only running on the server side. Ok, back to the drawing board then. |
|
I've included an idea for this API in a routing RFC that I just opened: sveltejs/rfcs#36 |
|
@benmccann I've read your RFC and looked at the code in @ehrencrona's PR and I'm not sure I'm convinced that this PR should be closed. Why not solve the issue that this code runs in both environments and then call this complete. The this.redirect() function calls code that only makes sense on the server. You can wrap the implementation of these header functions so that the client doesn't break. Being able to set cache-control and other headers like this in this context makes sense to me, sure there might be situations that using a middleware is more ideal for some header types but for the basics doing it in the context of the page you are rendering seems far better than in the router or other disconnected file to the page that you are rendering. |
Fixes #1354 by adding
getHeader(for request headers) andsetHeader(for response headers) to the preload context.A possible argument against this solution is that it allows less flexibility than just exposing the
reqandresobjects directly. Evidence of this would be that there are more header-related functions you might want such asgetHeaderNames.On the other hand I think this covers the vast majority of use cases and gives the user fewer possibilities to screw up.
It's also possible to argue that it's confusing that that
getHeaderoperates on request headers andsetHeaderon response headers, though, here too, in the vast majority of cases this is what you want. If you want to do anything more complex you should look into middleware.If the consensus is we should do this differently, I'm happy to change it.
(I also took the opportunity to extract the
fetchfunction used in preload into a different file, sinceget_page_handleris quite long and hard to overview)Before submitting the PR, please make sure you do the following
Tests
npm testand lint the project withnpm run lint