Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

in exporting, handle binary files from server routes #1398

Merged
merged 1 commit into from
Aug 12, 2020
Merged

in exporting, handle binary files from server routes #1398

merged 1 commit into from
Aug 12, 2020

Conversation

Conduitry
Copy link
Member

@Conduitry Conduitry commented Aug 12, 2020

Resolves #1103, resolves #1104. Builds on work in and supersedes #938 and #1105.

This is basically #1105, with conflicts with master resolved, code style adjusted, and with another case in get_server_route_handler of dangerous buffer-to-string conversion removed.

This lets you follow <img src>, <source src>, and <source srcset> attributes when exporting, and to also correctly handle (without encoding corruption) when these are server routes that return binary files.

- follow <img src>, <source src>, <source srcset> when crawling
- don't corrupt binary responses from server routes

Co-authored-by: ruben <[email protected]>
Co-authored-by: Andre Loker <[email protected]>
Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great. very thorough job testing!

@Conduitry Conduitry merged commit af33c09 into sveltejs:master Aug 12, 2020
@Conduitry Conduitry deleted the gh-1103-gh-1104 branch August 12, 2020 16:00
trmcnvn pushed a commit to metafy-gg/sapper that referenced this pull request Aug 15, 2020
- follow <img src>, <source src>, <source srcset> when crawling
- don't corrupt binary responses from server routes

Co-authored-by: ruben <[email protected]>
Co-authored-by: Andre Loker <[email protected]>
@HighestTide
Copy link

This currently results in kinda-expected, but kinda-annoying results if you have endpoints that return images without the route having the correct extension. Would be it be worth considering appending the "correct" extension for image files?

It is pretty simple to work around this by just adding the extension manually on the markup side and removing it in the route handler, but it seems like a simple fix could cover a lot of use cases and prevent confusion with exports not matching the running application. I'm not sure what the Sapper philosophy is when it comes to manipulating stuff like that, though.

Another alternative fix could just be to print a warning during export.

Example url based on the actual example I had:
/remote_asset/https%3A%2F%2Fexample.com%2Fasset.svg%3Fcompress%3D50
(https://example.com/asset.svg?compress=50)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sapper does not crawl img and source elements Exported server routes returning binary files get corrupted
3 participants