fix(nextjs): initialize new Date() with (0) to avoid breaking Next.js 16 Cache Components pre-render#82
Open
rikbrown wants to merge 1 commit intofullcalendar:mainfrom
Conversation
…he Components pre-renders
|
A change in |
Author
|
You're right, I was too broad in applying it here as it obviously is intended to get the current year. I am not completely sure the intention of this specific code, is it possible to do it in a way that does not require the current date? I traced it up (naively, didn't look in detail) and it looks like it's used in Instant.toString - it would be quite a bummer if we couldn't format a fixed Instant without needing to access the current date. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Next.js Cache Components does not allow access to any "dynamic" data during pre-rendering, because this is intended to build a static shell that could be cached by a remote server. It throws an error to prevent mistakes - i.e. make the user make a conscious decision what they want to do with dynamic data (in your Next app, you would mark it as cached for some period of time, in which case it will be cached in the pre-render).
new Date()is one of those functions you need to mark as explicitly cached and tell Next.js how long to cache it for (because it's the current time, not a fixed date).new Date('<some fixed>')would not be flagged as it is not dynamic.Unfortunately,
Temporal.Instant.from(<some fixed ISO string>)is flagged as dynamic, because it is usingnew Date()... but then immediately setting it to a fixed value inisoToLegacyDate. This forces users to wrap it with caching, which is tedious and unnecessary when it is actually a fixed date:This fix inits it with
new Date(0)instead, which stops Next.js complaining about this.note: also raised this to Next.js: vercel/next.js#88696 because it's a bit unergonomic that this pattern is flagged, although I can see why it'd be hard to detect.