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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 10, 2021
Merged

[Feauture] Support string arguments to utcOffset method #1395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
03f0436
feat: 🎸 Support utcOffset as a string (HH:mm | HHmm)
Mar 1, 2021
62d38f3
test: 💍 Test utcOffset string arguments usage
Mar 1, 2021
c99101a
chore: 🤖 Move utc related constants
Mar 2, 2021
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
27 changes: 27 additions & 0 deletions src/plugin/utc/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,26 @@
import { MILLISECONDS_A_MINUTE, MIN } from '../../constant'

export const REGEX_VALID_OFFSET_FORMAT = /[+-]\d\d(?::?\d\d)?/g
export const REGEX_OFFSET_HOURS_MINUTES_FORMAT = /([+-]|\d\d)/g

function offsetFromString(value = '') {
const offset = value.match(REGEX_VALID_OFFSET_FORMAT)
Copy link
Owner

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?

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 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?

Copy link
Owner

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?

Copy link
Contributor Author

@ognjenjevremovic ognjenjevremovic Mar 9, 2021 •
edited
Loading

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 🙂 .

Copy link
Owner

Choose a reason for hiding this comment

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

that make sense, thanks


if (!offset) {
return null
}

const [indicator, hoursOffset, minutesOffset] = `${offset[0]}`.match(REGEX_OFFSET_HOURS_MINUTES_FORMAT) || ['-', 0, 0]
const totalOffsetInMinutes = (+hoursOffset * 60) + (+minutesOffset)

if (totalOffsetInMinutes === 0) {
return 0
}

return indicator === '+' ? totalOffsetInMinutes : -totalOffsetInMinutes
}


export default (option, Dayjs, dayjs) => {
const proto = Dayjs.prototype
dayjs.utc = function (date) {
Expand Down Expand Up @@ -59,6 +80,12 @@ export default (option, Dayjs, dayjs) => {
}
return oldUtcOffset.call(this)
}
if (typeof input === 'string') {
input = offsetFromString(input)
if (input === null) {
return this
}
}
const offset = Math.abs(input) <= 16 ? input * 60 : input
let ins = this
if (keepLocalTime) {
Expand Down
65 changes: 65 additions & 0 deletions test/plugin/utc.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,71 @@ describe('UTC Offset', () => {
expect(dayjs().utcOffset()).toBe(moment().utcOffset())
expect(dayjs().utc().utcOffset()).toBe(moment().utc().utcOffset())
})

it('get utc offset with a number value', () => {
const time = '2021-02-28 19:40:10'
const hoursOffset = -8
const daysJS = dayjs(time).utc().utcOffset(hoursOffset * 60, true)
const momentJS = moment(time).utc(true).utcOffset(hoursOffset, true)

expect(daysJS.toISOString()).toEqual(momentJS.toISOString())
expect(daysJS.utcOffset()).toEqual(hoursOffset * 60)
expect(daysJS.utcOffset()).toEqual(momentJS.utcOffset())
})

it('get utc offset with a negative valid string value, format: HH:mm', () => {
const time = '2021-02-28 19:40:10'
const hoursOffset = -8
const daysJS = dayjs(time).utc().utcOffset(`-0${Math.abs(hoursOffset)}:00`, true)
const momentJS = moment(time).utc(true).utcOffset(`-0${Math.abs(hoursOffset)}:00`, true)

expect(daysJS.toISOString()).toEqual(momentJS.toISOString())
expect(daysJS.utcOffset()).toEqual(hoursOffset * 60)
expect(daysJS.utcOffset()).toEqual(momentJS.utcOffset())
})

it('get utc offset with a positive valid string value, format: HH:mm', () => {
const time = '2021-02-28 19:40:10'
const hoursOffset = 8
const daysJS = dayjs(time).utc().utcOffset(`+0${hoursOffset}:00`, true)
const momentJS = moment(time).utc(true).utcOffset(`+0${hoursOffset}:00`, true)

expect(daysJS.toISOString()).toEqual(momentJS.toISOString())
expect(daysJS.utcOffset()).toEqual(hoursOffset * 60)
expect(daysJS.utcOffset()).toEqual(momentJS.utcOffset())
})

it('get utc offset with a negative valid string value, format: HHmm', () => {
const time = '2021-02-28 19:40:10'
const hoursOffset = -8
const daysJS = dayjs(time).utc().utcOffset(`-0${Math.abs(hoursOffset)}00`, true)
const momentJS = moment(time).utc(true).utcOffset(`-0${Math.abs(hoursOffset)}00`, true)

expect(daysJS.toISOString()).toEqual(momentJS.toISOString())
expect(daysJS.utcOffset()).toEqual(hoursOffset * 60)
expect(daysJS.utcOffset()).toEqual(momentJS.utcOffset())
})

it('get utc offset with a positive valid string value, format: HHmm', () => {
const time = '2021-02-28 19:40:10'
const hoursOffset = 8
const daysJS = dayjs(time).utc().utcOffset(`+0${hoursOffset}00`, true)
const momentJS = moment(time).utc(true).utcOffset(`+0${hoursOffset}00`, true)

expect(daysJS.toISOString()).toEqual(momentJS.toISOString())
expect(daysJS.utcOffset()).toEqual(hoursOffset * 60)
expect(daysJS.utcOffset()).toEqual(momentJS.utcOffset())
})

it('get utc offset with an invalid string value, value: random', () => {
const time = '2021-02-28 19:40:10'
const daysJS = dayjs(time, { utc: true }).utc(true).utcOffset('random')
const momentJS = moment(time).utc(true).utcOffset('random')

expect(daysJS.toISOString()).toEqual(momentJS.toISOString())
expect(daysJS.utcOffset()).toEqual(0)
expect(daysJS.utcOffset()).toEqual(momentJS.utcOffset())
})
})

describe('Diff', () => {
Expand Down