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

Docs: Add note about Favicon Caching in Vercel #65755

Closed
wants to merge 1 commit into from

Conversation

columk1
Copy link

@columk1 columk1 commented May 14, 2024

What?

This PR adds a note to inform users of Vercel's build cache, which makes it difficult to replace favicon files once deployed.

Why?

There are many posts about issues replacing favicons on Stack Overflow, Reddit, Github etc. Many solutions involve renaming favicon.ico in the app folder to icon.ico, which works well but it's not clear why from what is said in the docs.

The docs don't mention that Vercel caches the favicon in the build cache and will not replace the file unless the build cache is invalidated or a different file is pointed to in the metadata.

Using any of the cache invalidation options at the link below will allow a favicon file to be updated after it has been deployed.

https://vercel.com/docs/deployments/troubleshoot-a-build#managing-build-cache

@columk1 columk1 requested review from a team as code owners May 14, 2024 18:35
@columk1 columk1 requested review from timeyoutakeit and leerob and removed request for a team May 14, 2024 18:35
@ijjk ijjk added the Documentation Related to Next.js' official documentation. label May 14, 2024
@ijjk
Copy link
Member

ijjk commented May 14, 2024

Allow CI Workflow Run

  • approve CI run for commit: 1a068a3

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

Copy link
Member

@leerob leerob left a comment

Choose a reason for hiding this comment

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

Is there a reproduction for this issue? So you are saying the file itself has changed, but it still cached?

@columk1
Copy link
Author

columk1 commented May 29, 2024

Is there a reproduction for this issue? So you are saying the file itself has changed, but it still cached?

Yes, I replicated it here using create-next-app:
https://github.com/columk1/test-next-favicon

When favicon.ico is deleted the Vercel deployment correctly has no favicon, but when a new favicon.ico is added, the old Vercel favicon re-appears.

There is more discussion here:
https://github.com/orgs/vercel/discussions/1808#discussioncomment-9426338

@samcx
Copy link
Member

samcx commented Jun 4, 2024

@columk1 I actually came across this issue myself! We should instead look to fix this issue instead of adding it as the expected behavior. We are taking a look!

@eps1lon
Copy link
Member

eps1lon commented Jun 4, 2024

Current thinking is that this is a Next.js bug not a Vercel bug. You should encounter the same problem on any hosting provider with a build cache.

@columk1
Copy link
Author

columk1 commented Jun 6, 2024

@columk1 I actually came across this issue myself! We should instead look to fix this issue instead of adding it as the expected behavior. We are taking a look!

Okay, that makes more sense. Thank you.

huozhi added a commit that referenced this pull request Jun 11, 2024
### What

Use `addDependency` to track the file path passed to
`next-metadata-route-loader`

NOTE: We cannot apply the `next-metadata-route-loader` directly to the
metatda convention source files, since the json file could be processed
by json loader (Related previous fix #62615)

### Why

Previously when we passed down the file path as argument to the loader,
which sort of breaking the caching of webpack as the actual resource
path is string, it's not tracked as a dependency. This change fixed the
bad caching issue of static metadata routes. Based on the above reason
we use `addDependency` here to track the dependency change

Closes NEXT-3521
Closes #65755
huozhi added a commit that referenced this pull request Jun 11, 2024
Use `addDependency` to track the file path passed to
`next-metadata-route-loader`

NOTE: We cannot apply the `next-metadata-route-loader` directly to the
metatda convention source files, since the json file could be processed
by json loader (Related previous fix #62615)

Previously when we passed down the file path as argument to the loader,
which sort of breaking the caching of webpack as the actual resource
path is string, it's not tracked as a dependency. This change fixed the
bad caching issue of static metadata routes. Based on the above reason
we use `addDependency` here to track the dependency change

Closes NEXT-3521
Closes #65755
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 27, 2024
huozhi added a commit that referenced this pull request Jul 9, 2024
Use `addDependency` to track the file path passed to
`next-metadata-route-loader`

NOTE: We cannot apply the `next-metadata-route-loader` directly to the
metatda convention source files, since the json file could be processed
by json loader (Related previous fix #62615)

Previously when we passed down the file path as argument to the loader,
which sort of breaking the caching of webpack as the actual resource
path is string, it's not tracked as a dependency. This change fixed the
bad caching issue of static metadata routes. Based on the above reason
we use `addDependency` here to track the dependency change

Closes NEXT-3521
Closes #65755
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Related to Next.js' official documentation. locked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants