-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[Feauture] Support string arguments to utcOffset
method
#1395
Conversation
Provide support string support to utcOffset method. Valid string values formats for utcOffset method are +HH:mm, -HH:mm, +HHmm and -HHmm. ✅ Closes: iamkun#1085
Extend the current suite of utc unit tests by providing unit tests for the utcOffset method with string arguments. ✅ Closes: iamkun#1085
UTC plugin constants should be self-contained from within the plugin package. ✅ Closes: iamkun#1395
export const REGEX_OFFSET_HOURS_MINUTES_FORMAT = /([+-]|\d\d)/g | ||
|
||
function offsetFromString(value = '') { | ||
const offset = value.match(REGEX_VALID_OFFSET_FORMAT) |
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.
can we skip these three lines, and put everything to REGEX_OFFSET_HOURS_MINUTES_FORMAT regex?
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 am not sure I understood you well.
Can you provide more info on the changes you would like regarding this code block?
Do you want the empty line removed and have the offsetFromString
method just below the REGEX_OFFSET_HOURS_MINUTES_FORMAT
, with no blank space in between?
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.
it seems REGEX_VALID_OFFSET_FORMAT and REGEX_OFFSET_HOURS_MINUTES_FORMAT is doing the same thing?
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.
Hey @iamkun the two provided regexes have some slight differences, but serve different purposes.
REGEX_VALID_OFFSET_FORMAT
is used to test the provided string value format.
If the method argument is not prefixed with -
or +
symbol the method exists early with null
value. That's the only purpose of this regex.
REGEX_OFFSET_HOURS_MINUTES_FORMAT
is used to split the output of the first regex into symbol, hours, minutes
variables. We need to split the string in order to provide the necessary calculations.
This could've been perhaps achieved using the String.prototype.slice
method instead, but would require multiple calls (3 calls to be precise in order to extract symbol, hours and minutes values) - regex might be faster in this case.
We could merge these two together but the method logic should be updated to reflect this change. It would break SR principle (more of a personal preference) and might be more error prone.
I am able to provide the requested changes, just wanted to give you a brief overview of the design choices behind 2 regex pattern 🙂 .
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.
that make sense, thanks
## [1.10.5](v1.10.4...v1.10.5) (2021-05-26) ### Bug Fixes * add meridiem in ar locales ([#1375](#1375)) ([319f616](319f616)) * Added Zulu support to customParseFormat ([#1359](#1359)) ([1138a3f](1138a3f)) * fix Bengali [bn] locale monthsShort error ([a0e6c0c](a0e6c0c)) * fix missing types for ArraySupport plugin ([#1401](#1401)) ([b1abdc4](b1abdc4)) * fix Ukrainian [uk] locale ([#1463](#1463)) ([0fdac93](0fdac93)) * hotfix for `Duration` types ([#1357](#1357)) ([855b7b3](855b7b3)), closes [#1354](#1354) * timezone plugin DST error ([#1352](#1352)) ([71bed15](71bed15)) * Update duration plugin change string to number ([#1394](#1394)) ([e1546d1](e1546d1)) * update Duration plugin to support no-argument ([#1400](#1400)) ([8d9a5ae](8d9a5ae)) * Update Finnish [fi] locale to set yearStart ([#1378](#1378)) ([f3370bd](f3370bd)) * update Russian [ru] locale meridiem and unit tests ([#1403](#1403)) ([f10f39d](f10f39d)) * update Russian [ru] locale yearStart config ([#1372](#1372)) ([5052515](5052515)) * update Slovenian [sl] locale to set correct ordinal ([#1386](#1386)) ([cb4f746](cb4f746)) * update Spanish [es] locale to change month names to lowercase ([#1414](#1414)) ([9c20e77](9c20e77)) * update Swedish [sv] locale to set correct yearStart ([#1385](#1385)) ([66c5935](66c5935)) * update UTC plugin to support string argument like +HH:mm ([#1395](#1395)) ([656127c](656127c))
🎉 This PR is included in version 1.10.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## [1.10.5](iamkun/dayjs@v1.10.4...v1.10.5) (2021-05-26) ### Bug Fixes * add meridiem in ar locales ([#1375](iamkun/dayjs#1375)) ([319f616](iamkun/dayjs@319f616)) * Added Zulu support to customParseFormat ([#1359](iamkun/dayjs#1359)) ([1138a3f](iamkun/dayjs@1138a3f)) * fix Bengali [bn] locale monthsShort error ([a0e6c0c](iamkun/dayjs@a0e6c0c)) * fix missing types for ArraySupport plugin ([#1401](iamkun/dayjs#1401)) ([b1abdc4](iamkun/dayjs@b1abdc4)) * fix Ukrainian [uk] locale ([#1463](iamkun/dayjs#1463)) ([0fdac93](iamkun/dayjs@0fdac93)) * hotfix for `Duration` types ([#1357](iamkun/dayjs#1357)) ([855b7b3](iamkun/dayjs@855b7b3)), closes [#1354](iamkun/dayjs#1354) * timezone plugin DST error ([#1352](iamkun/dayjs#1352)) ([71bed15](iamkun/dayjs@71bed15)) * Update duration plugin change string to number ([#1394](iamkun/dayjs#1394)) ([e1546d1](iamkun/dayjs@e1546d1)) * update Duration plugin to support no-argument ([#1400](iamkun/dayjs#1400)) ([8d9a5ae](iamkun/dayjs@8d9a5ae)) * Update Finnish [fi] locale to set yearStart ([#1378](iamkun/dayjs#1378)) ([f3370bd](iamkun/dayjs@f3370bd)) * update Russian [ru] locale meridiem and unit tests ([#1403](iamkun/dayjs#1403)) ([f10f39d](iamkun/dayjs@f10f39d)) * update Russian [ru] locale yearStart config ([#1372](iamkun/dayjs#1372)) ([5052515](iamkun/dayjs@5052515)) * update Slovenian [sl] locale to set correct ordinal ([#1386](iamkun/dayjs#1386)) ([cb4f746](iamkun/dayjs@cb4f746)) * update Spanish [es] locale to change month names to lowercase ([#1414](iamkun/dayjs#1414)) ([9c20e77](iamkun/dayjs@9c20e77)) * update Swedish [sv] locale to set correct yearStart ([#1385](iamkun/dayjs#1385)) ([66c5935](iamkun/dayjs@66c5935)) * update UTC plugin to support string argument like +HH:mm ([#1395](iamkun/dayjs#1395)) ([656127c](iamkun/dayjs@656127c))
## [1.10.5](iamkun/dayjs@v1.10.4...v1.10.5) (2021-05-26) ### Bug Fixes * add meridiem in ar locales ([#1375](iamkun/dayjs#1375)) ([319f616](iamkun/dayjs@319f616)) * Added Zulu support to customParseFormat ([#1359](iamkun/dayjs#1359)) ([1138a3f](iamkun/dayjs@1138a3f)) * fix Bengali [bn] locale monthsShort error ([a0e6c0c](iamkun/dayjs@a0e6c0c)) * fix missing types for ArraySupport plugin ([#1401](iamkun/dayjs#1401)) ([b1abdc4](iamkun/dayjs@b1abdc4)) * fix Ukrainian [uk] locale ([#1463](iamkun/dayjs#1463)) ([0fdac93](iamkun/dayjs@0fdac93)) * hotfix for `Duration` types ([#1357](iamkun/dayjs#1357)) ([855b7b3](iamkun/dayjs@855b7b3)), closes [#1354](iamkun/dayjs#1354) * timezone plugin DST error ([#1352](iamkun/dayjs#1352)) ([71bed15](iamkun/dayjs@71bed15)) * Update duration plugin change string to number ([#1394](iamkun/dayjs#1394)) ([e1546d1](iamkun/dayjs@e1546d1)) * update Duration plugin to support no-argument ([#1400](iamkun/dayjs#1400)) ([8d9a5ae](iamkun/dayjs@8d9a5ae)) * Update Finnish [fi] locale to set yearStart ([#1378](iamkun/dayjs#1378)) ([f3370bd](iamkun/dayjs@f3370bd)) * update Russian [ru] locale meridiem and unit tests ([#1403](iamkun/dayjs#1403)) ([f10f39d](iamkun/dayjs@f10f39d)) * update Russian [ru] locale yearStart config ([#1372](iamkun/dayjs#1372)) ([5052515](iamkun/dayjs@5052515)) * update Slovenian [sl] locale to set correct ordinal ([#1386](iamkun/dayjs#1386)) ([cb4f746](iamkun/dayjs@cb4f746)) * update Spanish [es] locale to change month names to lowercase ([#1414](iamkun/dayjs#1414)) ([9c20e77](iamkun/dayjs@9c20e77)) * update Swedish [sv] locale to set correct yearStart ([#1385](iamkun/dayjs#1385)) ([66c5935](iamkun/dayjs@66c5935)) * update UTC plugin to support string argument like +HH:mm ([#1395](iamkun/dayjs#1395)) ([656127c](iamkun/dayjs@656127c))
## [1.10.5](iamkun/dayjs@v1.10.4...v1.10.5) (2021-05-26) ### Bug Fixes * add meridiem in ar locales ([#1375](iamkun/dayjs#1375)) ([319f616](iamkun/dayjs@319f616)) * Added Zulu support to customParseFormat ([#1359](iamkun/dayjs#1359)) ([1138a3f](iamkun/dayjs@1138a3f)) * fix Bengali [bn] locale monthsShort error ([a0e6c0c](iamkun/dayjs@a0e6c0c)) * fix missing types for ArraySupport plugin ([#1401](iamkun/dayjs#1401)) ([b1abdc4](iamkun/dayjs@b1abdc4)) * fix Ukrainian [uk] locale ([#1463](iamkun/dayjs#1463)) ([0fdac93](iamkun/dayjs@0fdac93)) * hotfix for `Duration` types ([#1357](iamkun/dayjs#1357)) ([855b7b3](iamkun/dayjs@855b7b3)), closes [#1354](iamkun/dayjs#1354) * timezone plugin DST error ([#1352](iamkun/dayjs#1352)) ([71bed15](iamkun/dayjs@71bed15)) * Update duration plugin change string to number ([#1394](iamkun/dayjs#1394)) ([e1546d1](iamkun/dayjs@e1546d1)) * update Duration plugin to support no-argument ([#1400](iamkun/dayjs#1400)) ([8d9a5ae](iamkun/dayjs@8d9a5ae)) * Update Finnish [fi] locale to set yearStart ([#1378](iamkun/dayjs#1378)) ([f3370bd](iamkun/dayjs@f3370bd)) * update Russian [ru] locale meridiem and unit tests ([#1403](iamkun/dayjs#1403)) ([f10f39d](iamkun/dayjs@f10f39d)) * update Russian [ru] locale yearStart config ([#1372](iamkun/dayjs#1372)) ([5052515](iamkun/dayjs@5052515)) * update Slovenian [sl] locale to set correct ordinal ([#1386](iamkun/dayjs#1386)) ([cb4f746](iamkun/dayjs@cb4f746)) * update Spanish [es] locale to change month names to lowercase ([#1414](iamkun/dayjs#1414)) ([9c20e77](iamkun/dayjs@9c20e77)) * update Swedish [sv] locale to set correct yearStart ([#1385](iamkun/dayjs#1385)) ([66c5935](iamkun/dayjs@66c5935)) * update UTC plugin to support string argument like +HH:mm ([#1395](iamkun/dayjs#1395)) ([656127c](iamkun/dayjs@656127c))
✅ Closes: #1085
↪️ Pull Request
utcOffset
method with support for string argument representing the offset.Code example
Allow UTC offset to be defined as a string. Valid formats are:
Example usage:
✔️ PR Todo
📝 Additional notes
After the change I ensured that:
npm run test && npm run test-tz
,npm run lint
*.There are 2 failing unit tests present on the
dev
branch and as such are present in this fix as well, however they are not affected by this issue / code introduced.