Opened 7 years ago
Closed 5 years ago
#40657 closed enhancement (fixed)
Deprecate current_time( 'timestamp' )
Reported by: | jdgrimes | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Date/Time | Keywords: | has-patch |
Focuses: | Cc: |
Description
I've just opened #40653 to address some issues with how current_time()
operates internally. That ticket provides a bit of background on the age of the current_time()
function and why it does what it does.
In this ticket I'd like to specifically address the use of current_time( 'timestamp' )
, which 'gets the Unix timestamp of the current time in the site's timezone.'
The problem is that timestamps are actually timezoneless, they are UTC, meaning that they are always counting the number of seconds from the Unix epoch, which is a certain point in time, simultaneous across all timezones.
What WordPress is trying to do is explained by Otto here:
Basically, in order to maintain consistency across older PHP 4 installations, it was decided to have WP force the PHP environment to be using UTC dates for everything. [I.e.,
date()
is always GMT.] Then WordPress itself applies the adjustments when needed.
...
A call to current_time( 'timestamp' ) will give you the current timestamp adjusted by the necessary amount to get the correct time display. So to get the correct date in WordPress using date(), you'd do this:
date('Y-m-d', current_time( 'timestamp' ));
How it breaks down:
- PHP date and time functions should always give you the info in UTC.
- current_time( 'timestamp' ) will give you a UTC timestamp adjusted by the amount necessary to show the local date.
So this was deemed necessary for legacy reasons (PHP 4), but it is very confusing and tends to fool learning devs into a complete misunderstanding of timestamps, causing them to think that timestamps somehow have a timezone. In reality, this is just a quirk of legacy handling provided by WordPress. But it actually is no longer necessary, and probably ought to be deprecated, or at least discouraged.
I say that it isn't necessary because the current_time()
function now allows for any format string to be passed. This means that instead of this:
date('Y-m-d', current_time( 'timestamp' ));
You can skip the step of getting the "non-GMT" timestamp, and just do this:
current_time( 'Y-m-d' );
Also, because the timestamp does not actually carry any timezone information, if you do it the first way and try to format it later in a manner that includes timezone information, it will be UTC. (current_time()
actually has this same problem right now, but it would be fixed by #40653.)
In summary:
current_time( 'timestamp' )
is not necessary to use, the format you want to convert the timestamp to can be passed directly.- It confuses devs into thinking that timestamps are timezone-relative, when they are not.
- It won't work when ultimately converted to formats that include timezone information, since the timestamp does not actually include timezone info.
It might not be worth it to actually fully deprecate the 'timestamp'
arg when $gmt
is false, just because of the volume of legacy code that might be using this, but I thought at least I would open a ticket to raise awareness.
Attachments (2)
Change History (21)
This ticket was mentioned in Slack in #core by jdgrimes. View the logs.
7 years ago
#3
@
6 years ago
FWIW, I was one of those learning devs who completely misunderstood, and had to do massive changes and upgrade routines to fix it all.
I wonder if we could at least remove this out-dated instruction [here]https://codex.wordpress.org/Function_Reference/current_time:
current_time( 'timestamp' ) should be used in lieu of time() to return the blog's local time. In WordPress, PHP's time() will always return UTC and is the same as calling current_time( 'timestamp', true ).
#4
@
6 years ago
I’ve just recently started on that very major fixup of whole Date/Time component, so getting there. :)
current_time()
is clearly one of the most problematic functions (after date_i18n()
) and there are 6+ tickets about dealing with it. So I don't have a clear idea what are we going to do with it yet, but we definitely handle it in some way. It would be nice to deprecate it altogether, if possible.
#5
@
6 years ago
- Keywords has-patch added
Don't think we are at a point where we can deprecate timestamp use in core yet, but added a patch that eliminates most of the calls (down to literally three).
#6
@
6 years ago
- Milestone changed from Awaiting Review to 5.2
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
#9
@
6 years ago
Let me look into remainder of uses some more. I hadn't decided if they can be wrapped up here or worth handling in separate issues down the road.
#10
@
6 years ago
- Milestone changed from 5.2 to 5.3
As 5.2 Beta 1 approaches, moving to 5.3 for further discussion.
#11
@
5 years ago
- Keywords needs-patch added; has-patch removed
I've looked through remaining uses (5 or so) and I can (probably) clean them up once I get wp_timezone()
merged.
Though some of them are _very_ ambiguous, like getting timezone-less event date/time from API or fuzzy before/after in date queries.
#15
@
5 years ago
Down to last use in wp_post_revision_title_expanded()
! Will work on getting rid of it.
#16
@
5 years ago
- Keywords has-patch added; needs-patch removed
Added patch for last use that can be dropped.
Don't think we can formally deprecate it yet. Once last bit is merged I'll open WPCS issue to discourage the use for starters.
This ticket was mentioned in Slack in #core-datetime by rarst. View the logs.
5 years ago
#19
@
5 years ago
- Resolution set to fixed
- Status changed from reviewing to closed
Opened WPCS issue https://github.com/WordPress/WordPress-Coding-Standards/issues/1791
Going to close ticket as we are pretty much done with getting rid of it (as much as possible), but we can't formally deprecate it yet, with WP timestamps not really out of core completely.
We'll probably revisit deprecation in bulk after work on revamping the component is done and dust settles for a while.
Problem is some function expect this "WordPress timestamp" as input, notably
date_i18n()
. It cannot be dropped without major Date/Time component rewrite.This is the main problem with current state of the component, it just cannot be fixed in piecemeal fashion. Everything from storage, to computation, to output needs be replaced together since it's all linked and fuses functionality and bugs together.