Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(deno middleware): serve static assets in Deno middleware as binary data #4360

Merged
merged 1 commit into from
May 30, 2023
Merged

fix(deno middleware): serve static assets in Deno middleware as binary data #4360

merged 1 commit into from
May 30, 2023

Conversation

jaroel
Copy link
Contributor

@jaroel jaroel commented May 28, 2023

Overview

Fixes #4345

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Use cases and why

  • Safari didn't show images.

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

@stackblitz
Copy link

stackblitz bot commented May 28, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@jaroel
Copy link
Contributor Author

jaroel commented May 28, 2023

I've checked this change by applying it to an existing (previously failing) install. Please have someone actually check it as well and we should be grand.

@jaroel jaroel marked this pull request as ready for review May 28, 2023 11:31
@manucorporat
Copy link
Contributor

This is amazing!!! in order to check that it works, how do we do it? do you have some image that reproduced the issue?

Thinking to include that in our e2e test for deployments (which also tests Deno)(https://github.com/builderIO/qwik-city-e2e)

@jaroel
Copy link
Contributor Author

jaroel commented May 28, 2023

Importing any old image should do as all images were broken.
I have something along the lines of

import { component$ } from "@builder.io/qwik";
import theLogo from "~/assets/logos/thelogo.png";


export default component$(() => {
  return (
    <>
      <img
        src={theLogo}
        loading="eager"
        decoding="auto"
      />
    </>
  );
});

Then I ran pnpm run build and pnpm run serve. When using Deno it breaks images, Node works Just Fine(tm) :)

@manucorporat
Copy link
Contributor

I can indeed reproduce the issue, added a new e2e test!
QwikDev/qwik-city-e2e@8941dd4

@manucorporat manucorporat merged commit b5f28b6 into QwikDev:main May 30, 2023
19 checks passed
@manucorporat
Copy link
Contributor

Let's goo!! thank u so much @jaroel

@jaroel jaroel deleted the 4345-deno-static-readfile branch May 30, 2023 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deno middleware breaks binary artefacts in Safari
2 participants