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

refactor(about page): restore optimized images #1143

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

enapupe
Copy link
Contributor

@enapupe enapupe commented Jun 4, 2024

What type of PR is this?(check all applicable)

  • refactor
  • feature
  • bug fix
  • documentation
  • optimization
  • other

Description

Our image loader was not ready to handle static assets and they were 404. This PR is almost symbolic meaning we can now let static images route through the same loader as user-uploaded images.

Related Issues

Issue #1137

What this PR achieves

The PR itself only removes unoptimized prop from an image in About page, the actual change is happening in the background in the CF worker. The goal of this PR is actually test the worker.

previously, the cdn (media.openbeta.io) was not ready to process static assets
Copy link

vercel bot commented Jun 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
open-tacos ✅ Ready (Inspect) Visit Preview Jun 4, 2024 1:00pm

@enapupe
Copy link
Contributor Author

enapupe commented Jun 4, 2024

I've checked the about page and it's now loading static assets through the loader.
The worker now checks if the pathname starts with /_next/static and correctly selects the required base url.

@vnugent vnugent merged commit d1da49d into develop Jun 4, 2024
10 checks passed
@vnugent vnugent deleted the enapupe/restore-optimized branch June 4, 2024 19:01
@vnugent
Copy link
Contributor

vnugent commented Jun 7, 2024

@enapupe can you determine whether it's local dev env? Without unoptimized prop the image doesn't load in my local development environment.

@enapupe
Copy link
Contributor Author

enapupe commented Jun 7, 2024

@vnugent not really, cloudflare would not be able to reach it, the only option in this case is targeting localhost. I think this can be done just by having NEXT_PUBLIC_CDN_URL blank, right?

@enapupe
Copy link
Contributor Author

enapupe commented Jun 7, 2024

Oh, unless you want to point localhost images to staging bucket, in this case we'd have to adapt something. Maybe it's better to do it in the image-loader file? Not sure, let's discuss this.

If it's a static asset, you want to load it locally. If it's a user uploaded image from the google bucket then we want to load it from the staging bucket?

@vnugent
Copy link
Contributor

vnugent commented Jun 7, 2024

would something like this work?

<Image
  src={bouldering}
  width={700}
  unoptimized={ 'if local dev then true else false' }
 />

I experimented with image-loader for a bit. The moment you specify a custom loader for Image the about page becomes client component (required the 'use client' directive ). In the current Next framework server component is preferable.

https://nextjs.org/docs/app/building-your-application/rendering/client-components

@enapupe
Copy link
Contributor Author

enapupe commented Jun 7, 2024

Ideally this should be done at a top level and only once and for all, not at a component level. We already have a custom image loader in place, what do you mean by

The moment you specify a custom loader

?

Shall we talk about this on Discord?

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.

None yet

2 participants