Skip to content

Commit

Permalink
fix: switch to momentjs to handle timezones (opensearch-project#436)
Browse files Browse the repository at this point in the history
The current used time library, Luxon is using Intl.DateTimeFormatter as the main source for handling time zones. That library depends on browsers implementation. IE11 doesn't have any timezone and it will need a polyfill.
This PR replace luxon with moment-timezone as peerDependency. I'm leaving luxon as a devDependency on the tests and on stories.
  • Loading branch information
markov00 authored Oct 23, 2019
1 parent 1825a92 commit cd15932
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 31 deletions.
2 changes: 1 addition & 1 deletion packages/osd-charts/.playground/playgroud.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export class Playground extends React.Component<{}, { dataLimit: boolean }> {
yScaleType={ScaleType.Linear}
xAccessor={0}
yAccessors={[1]}
timeZone={'US/Pacific'}
data={data}
timeZone={'utc+8'}
/>
</Chart>
</div>
Expand Down
9 changes: 7 additions & 2 deletions packages/osd-charts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@
"@types/jest-environment-puppeteer": "^4.3.1",
"@types/jest-image-snapshot": "^2.8.0",
"@types/lodash": "^4.14.121",
"@types/luxon": "^1.11.1",
"@types/moment-timezone": "^0.5.12",
"@types/puppeteer": "^1.19.1",
"@types/react": "^16.8.5",
"@types/react-dom": "^16.8.2",
Expand Down Expand Up @@ -110,6 +112,8 @@
"jest-puppeteer": "^4.3.0",
"jest-puppeteer-docker": "^1.2.0",
"lorem-ipsum": "^2.0.3",
"luxon": "^1.11.3",
"moment-timezone": "^0.5.27",
"node-sass": "^4.11.0",
"postcss-cli": "^6.1.3",
"prettier": "1.16.4",
Expand All @@ -132,15 +136,13 @@
},
"dependencies": {
"@types/d3-shape": "^1.3.1",
"@types/luxon": "^1.11.1",
"classnames": "^2.2.6",
"d3-array": "^1.2.4",
"d3-collection": "^1.0.7",
"d3-scale": "^1.0.7",
"d3-shape": "^1.3.4",
"fp-ts": "^1.14.2",
"konva": "^2.6.0",
"luxon": "^1.11.3",
"mobx": "^4.9.2",
"mobx-react": "^5.4.3",
"newtype-ts": "^0.2.4",
Expand All @@ -153,6 +155,9 @@
"ts-debounce": "^1.0.0",
"uuid": "^3.3.2"
},
"peerDependencies": {
"moment-timezone": "^0.5.27"
},
"config": {
"commitizen": {
"path": "./node_modules/cz-conventional-changelog"
Expand Down
11 changes: 11 additions & 0 deletions packages/osd-charts/src/utils/data/date_time.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import moment from 'moment-timezone';

export function getMomentWithTz(date: number | Date, timeZone?: string) {
if (timeZone === 'local' || !timeZone) {
return moment(date);
}
if (timeZone.toLowerCase().startsWith('utc+') || timeZone.toLowerCase().startsWith('utc-')) {
return moment(date).utcOffset(Number(timeZone.slice(3)));
}
return moment.tz(date, timeZone);
}
25 changes: 10 additions & 15 deletions packages/osd-charts/src/utils/data/formatters.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,30 @@
import { DateTime, Interval } from 'luxon';
import { TickFormatter, TickFormatterOptions } from '../../chart_types/xy_chart/utils/specs';
import { getMomentWithTz } from './date_time';
import moment from 'moment-timezone';

export function timeFormatter(format: string): TickFormatter {
return (value: number, options?: TickFormatterOptions): string => {
const dateTimeOptions = options && options.timeZone ? { zone: options.timeZone } : undefined;
return DateTime.fromMillis(value, dateTimeOptions).toFormat(format);
return getMomentWithTz(value, options && options.timeZone).format(format);
};
}

export function niceTimeFormatter(domain: [number, number]): TickFormatter {
const minDate = DateTime.fromMillis(domain[0]);
const maxDate = DateTime.fromMillis(domain[1]);
const diff = Interval.fromDateTimes(minDate, maxDate);
const format = niceTimeFormat(diff);
const minDate = moment(domain[0]);
const maxDate = moment(domain[1]);
const diff = maxDate.diff(minDate, 'days');
const format = niceTimeFormatByDay(diff);
return timeFormatter(format);
}

export function niceTimeFormat(interval: Interval) {
const days = interval.length('days');
return niceTimeFormatByDay(days);
}

export function niceTimeFormatByDay(days: number) {
if (days > 30) {
return 'yyyy-MM-dd';
return 'YYYY-MM-DD';
}
if (days > 7 && days <= 30) {
return 'MMMM dd';
return 'MMMM DD';
}
if (days > 1 && days <= 7) {
return 'MM-dd HH:mm';
return 'MM-DD HH:mm';
}
return 'HH:mm:ss';
}
20 changes: 10 additions & 10 deletions packages/osd-charts/src/utils/scales/scale_continuous.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import {
ScalePower,
ScaleTime,
} from 'd3-scale';
import { DateTime } from 'luxon';

import { clamp, mergePartial } from '../commons';
import { ScaleContinuousType, ScaleType, Scale } from './scales';
import { getMomentWithTz } from '../data/date_time';

/**
* d3 scales excluding time scale
Expand Down Expand Up @@ -186,21 +186,21 @@ export class ScaleContinuous implements Scale {
this.totalBarsInCluster = totalBarsInCluster;
this.isSingleValueHistogram = isSingleValueHistogram;
if (type === ScaleType.Time) {
const startDomain = DateTime.fromMillis(this.domain[0], { zone: this.timeZone });
const endDomain = DateTime.fromMillis(this.domain[1], { zone: this.timeZone });
const offset = startDomain.offset;
const shiftedDomainMin = startDomain.plus({ minutes: offset }).toMillis();
const shiftedDomainMax = endDomain.plus({ minutes: offset }).toMillis();
const startDomain = getMomentWithTz(this.domain[0], this.timeZone);
const endDomain = getMomentWithTz(this.domain[1], this.timeZone);
const offset = startDomain.utcOffset();
const shiftedDomainMin = startDomain.add(offset, 'minutes').valueOf();
const shiftedDomainMax = endDomain.add(offset, 'minutes').valueOf();
const tzShiftedScale = scaleUtc().domain([shiftedDomainMin, shiftedDomainMax]);

const rawTicks = tzShiftedScale.ticks(ticks);
const timePerTick = (shiftedDomainMax - shiftedDomainMin) / rawTicks.length;
const hasHourTicks = timePerTick < 1000 * 60 * 60 * 12;

this.tickValues = rawTicks.map((d: Date) => {
const currentDateTime = DateTime.fromJSDate(d, { zone: this.timeZone });
const currentOffset = hasHourTicks ? offset : currentDateTime.offset;
return currentDateTime.minus({ minutes: currentOffset }).toMillis();
const currentDateTime = getMomentWithTz(d, this.timeZone);
const currentOffset = hasHourTicks ? offset : currentDateTime.utcOffset();
return currentDateTime.subtract(currentOffset, 'minutes').valueOf();
});
} else {
/**
Expand Down Expand Up @@ -241,7 +241,7 @@ export class ScaleContinuous implements Scale {
invert(value: number): number {
let invertedValue = this.d3Scale.invert(value);
if (this.type === ScaleType.Time) {
invertedValue = DateTime.fromJSDate(invertedValue as Date).toMillis();
invertedValue = getMomentWithTz(invertedValue, this.timeZone).valueOf();
}

return invertedValue as number;
Expand Down
5 changes: 4 additions & 1 deletion packages/osd-charts/wiki/consuming.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ To use Elastic Charts code in Kibana, check if `@elastic/charts` packages is alr

## Using Elastic Charts in a standalone project

You can consume Elastic Charts in standalone projects, such as plugins and prototypes.
You can consume Elastic Charts in standalone projects, such as plugins and prototypes. Elastic-Charts has a peer dependency on [moment-timezone](https://momentjs.com/timezone/). Add that dependency on your main project with:
```
yarn add moment-timezone
```

### Importing CSS

Expand Down
8 changes: 6 additions & 2 deletions packages/osd-charts/wiki/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,22 @@
Every jest test in this code runs in the local timezone.

### Run on every timezone

If you want to run a test suite on multiple timezone, just prepend a `tz` to the
standard `.test.ts` extension like:

```
formatters.tz.test.ts
scales.tz.test.ts
```

Your test will run in the standard local timezone and in all the configured timezones (UTC, America/New_York and Asia/Tokyo).
The local timezone tests results are included in the code coverage.

### Run on a specific timezone

If you are interested into explicitly create test for a timezone (we are now testing only on UTC, America/New_York and Asia/Tokyo) you have to prepend the `tz` but add one of the following postfix before the file extension: `utc`,`ny`,`jp`, for example

```sh
your_file_name.tz.test.utc.ts
your_file_name.tz.test.ny.ts
Expand All @@ -22,5 +27,4 @@ your_file_name.tz.test.jp.ts

Each test with the specific timezone postfix will be executed only on that timezone.

These tests are not included in the code coverage yet.

These tests are not included in the code coverage yet.
19 changes: 19 additions & 0 deletions packages/osd-charts/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2685,6 +2685,13 @@
resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.3.tgz#3dca0e3f33b200fc7d1139c0cd96c1268cadfd9d"
integrity sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA==

"@types/moment-timezone@^0.5.12":
version "0.5.12"
resolved "https://registry.yarnpkg.com/@types/moment-timezone/-/moment-timezone-0.5.12.tgz#0fb680c03db194fe8ff4551eaeb1eec8d3d80e9f"
integrity sha512-hnHH2+Efg2vExr/dSz+IX860nSiyk9Sk4pJF2EmS11lRpMcNXeB4KBW5xcgw2QPsb9amTXdsVNEe5IoJXiT0uw==
dependencies:
moment ">=2.14.0"

"@types/node@*":
version "12.0.2"
resolved "https://registry.yarnpkg.com/@types/node/-/node-12.0.2.tgz#3452a24edf9fea138b48fad4a0a028a683da1e40"
Expand Down Expand Up @@ -10743,6 +10750,18 @@ modify-values@^1.0.0:
resolved "https://registry.yarnpkg.com/modify-values/-/modify-values-1.0.1.tgz#b3939fa605546474e3e3e3c63d64bd43b4ee6022"
integrity sha512-xV2bxeN6F7oYjZWTe/YPAy6MN2M+sL4u/Rlm2AHCIVGfo2p1yGmBHQ6vHehl4bRTZBdHu3TSkWdYgkwpYzAGSw==

moment-timezone@^0.5.27:
version "0.5.27"
resolved "https://registry.yarnpkg.com/moment-timezone/-/moment-timezone-0.5.27.tgz#73adec8139b6fe30452e78f210f27b1f346b8877"
integrity sha512-EIKQs7h5sAsjhPCqN6ggx6cEbs94GK050254TIJySD1bzoM5JTYDwAU1IoVOeTOL6Gm27kYJ51/uuvq1kIlrbw==
dependencies:
moment ">= 2.9.0"

"moment@>= 2.9.0", moment@>=2.14.0:
version "2.24.0"
resolved "https://registry.yarnpkg.com/moment/-/moment-2.24.0.tgz#0d055d53f5052aa653c9f6eb68bb5d12bf5c2b5b"
integrity sha512-bV7f+6l2QigeBBZSM/6yTNq4P2fNpSWj/0e7jQcy87A8e7o2nAfP/34/2ky5Vw4B9S446EtIhodAzkFCcR4dQg==

monocle-ts@^1.0.0:
version "1.7.2"
resolved "https://registry.yarnpkg.com/monocle-ts/-/monocle-ts-1.7.2.tgz#d9825ae18846ab63f915cb6f2194a78a40025610"
Expand Down

0 comments on commit cd15932

Please sign in to comment.