solidjs / solid-start

SolidStart, the Solid app framework
https://start.solidjs.com
MIT License
4.93k stars 371 forks source link

Return result verbatim if `raw=true` is provided in the query string #1553

Closed Tommypop2 closed 1 week ago

Tommypop2 commented 1 week ago

PR Type

What kind of change does this PR introduce?

What is the current behavior?

Described in #1550

What is the new behavior?

Can now specify that the response is returned verbatim by adding ?raw=true to the query string

Closes #1550

changeset-bot[bot] commented 1 week ago

⚠️ No Changeset found

Latest commit: 49397549f46557bb1198d80786d42aa871bd262c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

ryansolid commented 1 week ago

So if i get this right.. this is for something like an image tag.. so it falls into the no js case but our setting of flash headers messes things.. or I guess more so us messing with the body encoding with seroval would be the issue. I don't like having to rely on query param.. but I also don't see a particularly great way to handle this in this case.

Our options are to use the query param as you have if the caller makes that decision. Or to do something on the response, if the response makes that decision. It sort of feels like the response knows what it wants sent back. We could use the content-type headers of the response object maybe to make this decision. I guess that is a bit weird.

I'm just unclear all the scenarios here and before adding a convention I'd like to consider them.

Brendonovich commented 1 week ago

IMO we should use the Accept request header here instead, just to account for the case where seroval is wanted. If Accept has text/javascript then seroval is used, otherwise it's returned verbatim (unless it's a redirect ofc)

OrJDev commented 1 week ago

So if i get this right.. this is for something like an image tag.. so it falls into the no js case but our setting of flash headers messes things.. or I guess more so us messing with the body encoding with seroval would be the issue. I don't like having to rely on query param.. but I also don't see a particularly great way to handle this in this case.

Our options are to use the query param as you have if the caller makes that decision. Or to do something on the response, if the response makes that decision. It sort of feels like the response knows what it wants sent back. We could use the content-type headers of the response object maybe to make this decision. I guess that is a bit weird.

I'm just unclear all the scenarios here and before adding a convention I'd like to consider them.

How about a response header

X-Solid-Start-Raw: 1
ryansolid commented 1 week ago

Yeah the problem with accept headers is things like image URLs can't set them I believe. So yeah response header seems the best option to me. It means being a bit more explicit in these cases but I think that makes sense. The only real question is if we just set one for raw or we leave it open. I guess raw is fine and then rely on the native body parser to handle the rest. We do have to consider both the nojs and js case though so its going to take a bit more wiring. I will take a stab it today I think.

ryansolid commented 1 week ago

I went with going with "X-Content-Raw". So not going with this PR. Thanks for bring this up to my attention.