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

Avif server as octet-stream #39706

Closed
1 task done
pegak opened this issue Aug 18, 2022 · 8 comments · Fixed by #39733
Closed
1 task done

Avif server as octet-stream #39706

pegak opened this issue Aug 18, 2022 · 8 comments · Fixed by #39733
Labels
template: bug A user has filled out the bug report template. Issue needs triaging

Comments

@pegak
Copy link

pegak commented Aug 18, 2022

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Same problem as for Gatsby: gatsbyjs/gatsby#31068
Or previously Remix: remix-run/remix#3771

What browser are you using? (if relevant)

Chrome 104, Edge 106

How are you deploying your application? (if relevant)

Vercel

Describe the Bug

Wrong mime type for AVIF image from public folder.

Expected Behavior

Correct Content-Type, so image can be loaded.

Link to reproduction

https://github.com/pegak/nauc-me-it - you need to have pixel density 2x, run locally and check bg-landing.avif loading :)

To Reproduce

  1. Put AVIF image into public folder.
  2. Use that image (I use it from CSS as a background image).
  3. Image does not work.
  4. Check Network tab in Devtools.
  5. Image is loaded, but with type octet-stream.
@pegak pegak added the template: bug A user has filled out the bug report template. Issue needs triaging label Aug 18, 2022
@SukkaW
Copy link
Contributor

SukkaW commented Aug 18, 2022

That's really weird.

image

I just use my browser to visit the URL directly, and the response header shows image/avif.

@pegak
Copy link
Author

pegak commented Aug 18, 2022

And when you run it locally via yarn, yarn run dev? I think it is broken even when running yarn run build && yarn run start.

@SukkaW
Copy link
Contributor

SukkaW commented Aug 18, 2022

And when you run it locally via yarn, yarn run dev? I think it is broken even when running yarn run build && yarn run start.

@pegak Yes. I am able to reproduce the issue in local environment.

export function getContentType(extWithoutDot: string): string | null {
if (extWithoutDot === 'avif') {
// TODO: update "mime" package
return 'image/avif'
}
const { mime } = send

Next.js already implemented .avif handling (as a specific edge case). I will have a look at it to see what went wrong.

@SukkaW
Copy link
Contributor

SukkaW commented Aug 18, 2022

@pegak I have submitted a PR that will fix the issue: #39733

@pegak
Copy link
Author

pegak commented Aug 18, 2022

Thank you very much! I must make some time to contribute to open source community to give something back! :)

@kodiakhq kodiakhq bot closed this as completed in #39733 Aug 18, 2022
kodiakhq bot pushed a commit that referenced this issue Aug 18, 2022
The PR fixes #39706 by adding `avif` mime type directly to `send`. The PR also removes the previous avif workaround for image optimizer.

Note: The PR is still a workaround for now. I will submit a PR to `pillarjs/send` to help them update `mime` to fix the issue once and for all. But now `send.mime.define` just works.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
@SukkaW
Copy link
Contributor

SukkaW commented Aug 18, 2022

Thank you very much! I must make some time to contribute to open source community to give something back! :)

@pegak Actually now you have a great opportunity to start with!

Your issue is caused by an upstream package send. It depends on another package mime to provide the content-type mapping.

The mime package has added avif in mime@2.4.7, but send is still using mime@1.6.0, hence your issue. You can submit a PR to send to help them update the mime dependency version!

A few notes:

  • It seems that send is about to release a major version 1.0.0, thus there might be a dedicated developing branch. If there is a dedicated branch for the upcoming send@1.0.0, you should create your PR toward that branch, not master.
  • From mime@1.6.0 to mime@2 is a major version bump, and mime@2 has introduced some breaking changes to its API, thus you should also help them to migrate.

@pegak
Copy link
Author

pegak commented Aug 19, 2022

Thanks, looks like they already switched to mime-types, but thanks for the recommendation anyway. :)

@github-actions
Copy link

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
template: bug A user has filled out the bug report template. Issue needs triaging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants