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

date: Add types #29789

Merged
merged 2 commits into from
Mar 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions packages/date/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ _Related_
_Parameters_

- _dateFormat_ `string`: PHP-style formatting string. See php.net/date.
- _dateValue_ `Date|string|Moment|null`: Date object or string, parsable by moment.js.
- _timezone_ `string|number|null`: Timezone to output result in or a UTC offset. Defaults to timezone from site.
- _dateValue_ `Moment | Date | string | undefined`: Date object or string, parsable by moment.js.
- _timezone_ `string | undefined`: Timezone to output result in or a UTC offset. Defaults to timezone from site.

_Returns_

Expand All @@ -50,8 +50,8 @@ _Related_
_Parameters_

- _dateFormat_ `string`: PHP-style formatting string. See php.net/date.
- _dateValue_ `Date|string|Moment|null`: Date object or string, parsable by moment.js.
- _timezone_ `string|number|boolean|null`: Timezone to output result in or a UTC offset. Defaults to timezone from site. Notice: `boolean` is effectively deprecated, but still supported for backward compatibility reasons.
- _dateValue_ `Moment | Date | string | undefined`: Date object or string, parsable by moment.js.
- _timezone_ `string | boolean | undefined`: Timezone to output result in or a UTC offset. Defaults to timezone from site. Notice: `boolean` is effectively deprecated, but still supported for backward compatibility reasons.

_Returns_

Expand All @@ -64,7 +64,7 @@ Formats a date. Does not alter the date's timezone.
_Parameters_

- _dateFormat_ `string`: PHP-style formatting string. See php.net/date.
- _dateValue_ `Date|string|Moment|null`: Date object or string, parsable by moment.js.
- _dateValue_ `Moment | Date | string | undefined`: Date object or string, parsable by moment.js.

_Returns_

Expand All @@ -89,7 +89,7 @@ Formats a date (like `date()` in PHP), in the UTC timezone.
_Parameters_

- _dateFormat_ `string`: PHP-style formatting string. See php.net/date.
- _dateValue_ `Date|string|Moment|null`: Date object or string, parsable by moment.js.
- _dateValue_ `Moment | Date | string | undefined`: Date object or string, parsable by moment.js.

_Returns_

Expand All @@ -103,7 +103,7 @@ and using the UTC timezone.
_Parameters_

- _dateFormat_ `string`: PHP-style formatting string. See php.net/date.
- _dateValue_ `Date|string|Moment|null`: Date object or string, parsable by moment.js.
- _dateValue_ `Moment | Date | string | undefined`: Date object or string, parsable by moment.js.

_Returns_

Expand All @@ -127,7 +127,7 @@ Adds a locale to moment, using the format supplied by `wp_localize_script()`.

_Parameters_

- _dateSettings_ `Object`: Settings, including locale data.
- _dateSettings_ `DateSettings`: Settings, including locale data.


<!-- END TOKEN(Autogenerated API docs) -->
Expand Down
106 changes: 79 additions & 27 deletions packages/date/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,50 @@ import 'moment-timezone/moment-timezone';
import 'moment-timezone/moment-timezone-utils';

/** @typedef {import('moment').Moment} Moment */
/** @typedef {import('moment').LocaleSpecification} MomentLocaleSpecification */

/**
* @typedef MeridiemConfig
* @property {string} am Lowercase AM.
* @property {string} AM Uppercase AM.
* @property {string} pm Lowercase PM.
* @property {string} PM Uppercase PM.
*/

/**
* @typedef FormatsConfig
* @property {string} time Time format.
* @property {string} date Date format.
* @property {string} datetime Datetime format.
* @property {string} datetimeAbbreviated Abbreviated datetime format.
*/

/**
* @typedef TimezoneConfig
* @property {string} offset Offset setting.
* @property {string} string The timezone as a string (e.g., `'America/Los_Angeles'`).
* @property {string} abbr Abbreviation for the timezone.
*/

/* eslint-disable jsdoc/valid-types */
/**
* @typedef L10nSettings
* @property {string} locale Moment locale.
* @property {MomentLocaleSpecification['months']} months Locale months.
* @property {MomentLocaleSpecification['monthsShort']} monthsShort Locale months short.
* @property {MomentLocaleSpecification['weekdays']} weekdays Locale weekdays.
* @property {MomentLocaleSpecification['weekdaysShort']} weekdaysShort Locale weekdays short.
Copy link
Member

Choose a reason for hiding this comment

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

There are a bunch more locale specification properties available. Should we include more as available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only included the ones that are actually used in the settings object and in the setSettings function. Were the specific other ones you think we should include for some reason even though they're not used?

Copy link
Member

Choose a reason for hiding this comment

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

I think that's a good start 👍 The only problem would be if someone uses some of the other ones and gets TS errors. Those could be added incrementally, though - this isn't something that should be blocking this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh hmm that's interesting. I guess someone could be adding extraneous properties? But then shouldn't they indeed get an error considering those properties aren't supported by the code? Or should we extend this as a Record<string, any>?

Copy link
Member

Choose a reason for hiding this comment

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

Well, the thing is that they're supported by Moment, so practically they're also supported by the code. The typed ones we're adding here are only the default settings, right?

Anyway, we could make this a Record, but I believe would be much more precise if we just borrow the rest of the LocaleSpecification interface.

I'll leave it to you to decide the level of precision you'd like to achieve with this first PR - I'm fine with what we have as a start already 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The typed ones we're adding here are only the default settings, right

Nope, if you check out the setSettings function I only added the properties we actually access off the l10n object.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha 👍

Well, I don't see a good reason to add more in that case.

* @property {MeridiemConfig} meridiem Meridiem config.
* @property {MomentLocaleSpecification['relativeTime']} relative Relative time config.
*/
/* eslint-enable jsdoc/valid-types */

/**
* @typedef DateSettings
* @property {L10nSettings} l10n Localization settings.
* @property {FormatsConfig} formats Date/time formats config.
* @property {TimezoneConfig} timezone Timezone settings.
*/

const WP_ZONE = 'WP';

Expand All @@ -15,6 +59,7 @@ const VALID_UTC_OFFSET = /^[+-][0-1][0-9](:?[0-9][0-9])?$/;

// Changes made here will likely need to be made in `lib/client-assets.php` as
// well because it uses the `setSettings()` function to change these settings.
/** @type {DateSettings} */
let settings = {
l10n: {
locale: 'en',
Expand Down Expand Up @@ -86,7 +131,7 @@ let settings = {
/**
* Adds a locale to moment, using the format supplied by `wp_localize_script()`.
*
* @param {Object} dateSettings Settings, including locale data.
* @param {DateSettings} dateSettings Settings, including locale data.
*/
export function setSettings( dateSettings ) {
settings = dateSettings;
Expand All @@ -112,10 +157,13 @@ export function setSettings( dateSettings ) {
},
longDateFormat: {
LT: dateSettings.formats.time,
// @ts-ignore Forcing this to `null`
LTS: null,
// @ts-ignore Forcing this to `null`
L: null,
LL: dateSettings.formats.date,
LLL: dateSettings.formats.datetime,
// @ts-ignore Forcing this to `null`
LLLL: null,
},
// From human_time_diff?
Expand Down Expand Up @@ -177,8 +225,6 @@ const HOUR_IN_SECONDS = 60 * MINUTE_IN_SECONDS;
*
* This should only be used through {@link wp.date.format}, not
* directly.
*
* @type {Object}
*/
const formatMap = {
// Day
Expand Down Expand Up @@ -212,7 +258,7 @@ const formatMap = {
*/
z( momentDate ) {
// DDD - 1
return '' + parseInt( momentDate.format( 'DDD' ), 10 ) - 1;
return ( parseInt( momentDate.format( 'DDD' ), 10 ) - 1 ).toString();
Copy link
Member

Choose a reason for hiding this comment

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

👍

},

// Week
Expand All @@ -228,7 +274,7 @@ const formatMap = {
*
* @param {Moment} momentDate Moment instance.
*
* @return {string} Formatted date.
* @return {number} Formatted date.
*/
t( momentDate ) {
return momentDate.daysInMonth();
Expand Down Expand Up @@ -257,18 +303,20 @@ const formatMap = {
*
* @param {Moment} momentDate Moment instance.
*
* @return {string} Formatted date.
* @return {number} Formatted date.
*/
B( momentDate ) {
const timezoned = momentLib( momentDate ).utcOffset( 60 );
const seconds = parseInt( timezoned.format( 's' ), 10 ),
minutes = parseInt( timezoned.format( 'm' ), 10 ),
hours = parseInt( timezoned.format( 'H' ), 10 );
return parseInt(
( seconds +
minutes * MINUTE_IN_SECONDS +
hours * HOUR_IN_SECONDS ) /
86.4,
(
( seconds +
minutes * MINUTE_IN_SECONDS +
hours * HOUR_IN_SECONDS ) /
86.4
).toString(),
Copy link
Member

Choose a reason for hiding this comment

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

Looks safe, parseInt() will implicitly convert to string before parsing to an integer, so it's even better if we do it explicitly.

10
);
},
Expand Down Expand Up @@ -300,13 +348,16 @@ const formatMap = {
*
* @param {Moment} momentDate Moment instance.
*
* @return {string} Formatted date.
* @return {number} Formatted date.
*/
Z( momentDate ) {
// Timezone offset in seconds.
const offset = momentDate.format( 'Z' );
const sign = offset[ 0 ] === '-' ? -1 : 1;
const parts = offset.substring( 1 ).split( ':' );
const parts = offset
.substring( 1 )
.split( ':' )
.map( ( n ) => parseInt( n, 10 ) );
Copy link
Member

Choose a reason for hiding this comment

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

The explicit conversion looks much better than the one we were implicitly doing with multiplication and addition below 👍

return (
sign *
( parts[ 0 ] * HOUR_IN_MINUTES + parts[ 1 ] ) *
Expand All @@ -324,14 +375,14 @@ const formatMap = {
*
* @param {string} dateFormat PHP-style formatting string.
* See php.net/date.
* @param {Date|string|Moment|null} dateValue Date object or string,
* @param {Moment | Date | string | undefined} dateValue Date object or string,
* parsable by moment.js.
*
* @return {string} Formatted date.
*/
export function format( dateFormat, dateValue = new Date() ) {
let i, char;
let newFormat = [];
const newFormat = [];
const momentDate = momentLib( dateValue );
for ( i = 0; i < dateFormat.length; i++ ) {
char = dateFormat[ i ];
Expand All @@ -343,31 +394,32 @@ export function format( dateFormat, dateValue = new Date() ) {
continue;
}
if ( char in formatMap ) {
if ( typeof formatMap[ char ] !== 'string' ) {
const formatter =
formatMap[ /** @type {keyof formatMap} */ ( char ) ];
if ( typeof formatter !== 'string' ) {
// If the format is a function, call it.
newFormat.push( '[' + formatMap[ char ]( momentDate ) + ']' );
newFormat.push( '[' + formatter( momentDate ) + ']' );
} else {
// Otherwise, add as a formatting string.
newFormat.push( formatMap[ char ] );
newFormat.push( formatter );
}
} else {
newFormat.push( '[' + char + ']' );
}
}
// Join with [] between to separate characters, and replace
// unneeded separators with static text.
newFormat = newFormat.join( '[]' );
return momentDate.format( newFormat );
return momentDate.format( newFormat.join( '[]' ) );
}

/**
* Formats a date (like `date()` in PHP).
*
* @param {string} dateFormat PHP-style formatting string.
* See php.net/date.
* @param {Date|string|Moment|null} dateValue Date object or string, parsable
* @param {Moment | Date | string | undefined} dateValue Date object or string, parsable
* by moment.js.
* @param {string|number|null} timezone Timezone to output result in or a
* @param {string | undefined} timezone Timezone to output result in or a
* UTC offset. Defaults to timezone from
* site.
*
Expand All @@ -386,7 +438,7 @@ export function date( dateFormat, dateValue = new Date(), timezone ) {
*
* @param {string} dateFormat PHP-style formatting string.
* See php.net/date.
* @param {Date|string|Moment|null} dateValue Date object or string,
* @param {Moment | Date | string | undefined} dateValue Date object or string,
* parsable by moment.js.
*
* @return {string} Formatted date in English.
Expand All @@ -404,9 +456,9 @@ export function gmdate( dateFormat, dateValue = new Date() ) {
*
* @param {string} dateFormat PHP-style formatting string.
* See php.net/date.
* @param {Date|string|Moment|null} dateValue Date object or string, parsable by
* @param {Moment | Date | string | undefined} dateValue Date object or string, parsable by
* moment.js.
* @param {string|number|boolean|null} timezone Timezone to output result in or a
* @param {string | boolean | undefined} timezone Timezone to output result in or a
* UTC offset. Defaults to timezone from
* site. Notice: `boolean` is effectively
* deprecated, but still supported for
Expand Down Expand Up @@ -437,7 +489,7 @@ export function dateI18n( dateFormat, dateValue = new Date(), timezone ) {
*
* @param {string} dateFormat PHP-style formatting string.
* See php.net/date.
* @param {Date|string|Moment|null} dateValue Date object or string,
* @param {Moment | Date | string | undefined} dateValue Date object or string,
* parsable by moment.js.
*
* @return {string} Formatted date.
Expand Down Expand Up @@ -480,9 +532,9 @@ export function getDate( dateString ) {
/**
* Creates a moment instance using the given timezone or, if none is provided, using global settings.
*
* @param {Date|string|Moment|null} dateValue Date object or string, parsable
* @param {Moment | Date | string | undefined} dateValue Date object or string, parsable
* by moment.js.
* @param {string|number|null} timezone Timezone to output result in or a
* @param {string | undefined} timezone Timezone to output result in or a
* UTC offset. Defaults to timezone from
* site.
*
Expand Down
9 changes: 9 additions & 0 deletions packages/date/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"rootDir": "src",
"declarationDir": "build-types",
"noUnusedParameters": false
},
"include": [ "src/**/*" ]
}
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
{ "path": "packages/blob" },
{ "path": "packages/block-editor" },
{ "path": "packages/components" },
{ "path": "packages/date" },
{ "path": "packages/deprecated" },
{ "path": "packages/docgen" },
{ "path": "packages/dom" },
Expand Down