-
-
Notifications
You must be signed in to change notification settings - Fork 294
feat: Use numeric: 'auto'
for relative times that don't need to be rounded for a given unit
#1872
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
numeric: 'auto'
for relative times that don't need to be roundednumeric: 'auto'
for relative times that don't need to be rounded for a given unit
now: normalize(now), | ||
unit: 'day' | ||
}); | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Been scratching my head a bit over this code snippet. I think it should now be correct, but it's a bit verbose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is technically what I had to do anyways with my current approach (with the exception of using Luxon instead of date-fns) to ensure that numeric: auto
gave the proper localized text for "yesterday", "today", etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if there's a way to simplify this, unless we allow passing in an optional config param like shouldNormalizeDate: boolean
to the format.relativeTime()
options - but that gets into the issue of requiring next-intl
to supply its own "start of day" function calculation if it wants to be date-library agnostic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question is also what to normalize to. The start of the day is an example here, but you might normalize to a week (e.g. "last week"). I don't think we can make an assumption here.
// Note that due to approximations being used for months and years, it's | ||
// practically impossible to trigger the cases "last month" or "last year". | ||
if (unitValue === rounded) { | ||
opts.numeric = 'auto'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this always override to numeric: auto
whenever possible? I'm all for supporting numeric: auto
but not sure if it's a good idea for us to always try to coerce into "auto" and at least provide the user the flexibility to opt out as a custom configuration they can pass in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of having this as the default, but we should give the flexibility to allow the user to opt out if necessary. I think that might be better, rather than trying to expend effort to figure out a justification of why we shouldn't do it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also unsure what the best way really is here.
Some aspects to consider:
Differences to Intl.RelativeTimeFormat
format.relativeTime
is not just "use Intl.RelativeTime
with the current locale".
Instead, it's a function to produce readable relative time output and internally does:
- Calculate a time difference
- Detect an appropriate unit (can be overridden)
- Round to the nearest value based on the unit
- Apply the locale to a cached formatter
So I think some deviations from Intl.RelativeTime
might be fine. Generally, I'd like the function to produce a readable output for the consumer with the least amount of config and edge cases to consider.
Using numeric: 'auto'
is quite tricky
It requires date calculations on the consumer side, also taking into account the current time zone. If the user would be able to simply pass numeric: 'auto'
, they might see a result like "yesterday" and will push to production.
It could break in other cases though, due to the rounding that next-intl
does. We could add a big disclaimer in the docs, but users might miss this if they discover the option in their IDE via IntelliSense.
Generally, it's quite a lot of effort to support cases like "yesterday" (see the code snippet in the docs).
Which special cases does CLDR provide?
CLDR seems to mostly has special cases for -1
, 0
, 1
for auto
.
<relative type="-1">yesterday</relative>
<relative type="0">today</relative>
<relative type="1">tomorrow</relative>
Maybe it would help to create a script that extracts all currently known cases to get a better idea what auto
can produce.
My main question is: Are there cases where auto
would produce an output that is not desired? That's the point you've raised above, and I'm also wondering if that might be the case.
auto
doesn't work for months, quarters and years
Due to an approximation that next-intl
has to make, it's practically impossible for users to trigger auto
special cases for this.
If this is the output you need, you might be better off using Intl.RelativeTime
directly.
Format.js allows passing auto
FormattedRelativeTime allows to set numeric: 'auto'
.
It's interesting to see, but I really wonder how many times users run into issues like incorrectly rounded values. They don't have any validation against rounded values, so they make it really easy to trigger cases like "yesterday", but this will break in many cases. I'm not sure if this is a reasonable solution that I'd like to provide.
So there are still some open questions here for me and I'm not really confident what the best solution is.
Another option would be to provide a separate function that mimics Intl.RelativeTime
closely and only applies the locale (and also benefits from internal caching).
Example:
format.relativeTimeValue(1, 'day', {numeric: 'auto'});
This would put more work into the hand of a user, but gives you all the flexibility. It's almost the same as using Intl.RelativeTimeFormat
yourself though (which is what you're doing currently)—not sure if it's worth adding and maintaining this.
So these are my current thoughts. I created this PR to explore the topic in more depth, but I didn't really come to a place yet where my questions are cleared up unfortunately …
What's your position?
This allows to use idiomatic phrasing like "yesterday" in case a date is provided that doesn't have a fractional part when converted to the target unit.
See the updated docs.
(ref)