Skip to content

Commit

Permalink
[dagit] Replace remaining moment usage (#11278)
Browse files Browse the repository at this point in the history
### Summary & Motivation

Asset code still has a handful of `moment` references. Remove them in favor of dayjs.

Also add linting against imports of `moment` and `moment-timezone`.

### How I Tested These Changes

I'm not sure how to load up assets that will give me these strings, and I'm also not sure there is test coverage for them. Probably need some help here.
  • Loading branch information
hellendag authored Dec 20, 2022
1 parent eca24df commit 5d83868
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ type Config = {
timeFormat?: TimeFormat;
};

// This helper is here so that we can swap out Moment in the future as needed and
// encourage use of the same default format string across the app.
export const timestampToString = (config: Config) => {
const {timestamp, locale, timezone, timeFormat = DEFAULT_TIME_FORMAT} = config;
const msec = 'ms' in timestamp ? timestamp.ms : timestamp.unix * 1000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
Table,
Mono,
} from '@dagster-io/ui';
import moment from 'moment';
import dayjs from 'dayjs';
import * as React from 'react';
import {Link} from 'react-router-dom';
import styled from 'styled-components/macro';
Expand Down Expand Up @@ -132,9 +132,9 @@ const MetadataEntriesRow: React.FC<{
<Link to={`/runs/${obs.runId}?timestamp=${obs.timestamp}`}>
<Mono>{titleForRun({runId: obs.runId})}</Mono>
</Link>
{` (${moment(Number(obs.timestamp)).from(
Number(timestamp),
true,
{` (${dayjs(obs.timestamp).from(
timestamp,
true, // withoutSuffix
)} later)`}
</span>
</Box>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Box, Caption, Colors, Icon, Mono} from '@dagster-io/ui';
import moment from 'moment';
import dayjs from 'dayjs';
import React from 'react';
import {Link} from 'react-router-dom';
import styled from 'styled-components/macro';
Expand Down Expand Up @@ -65,7 +65,7 @@ export const AssetEventMetadataEntriesTable: React.FC<{
</span>
</Box>
<Caption style={{marginLeft: 24}}>
{` (${moment(Number(obs.timestamp)).from(Number(timestamp), true)} later)`}
{`(${dayjs(obs.timestamp).from(timestamp, true /* withoutSuffix */)} later)`}
</Caption>
{entry.description}
</td>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {Tooltip, Tag} from '@dagster-io/ui';
import moment from 'moment';
import dayjs from 'dayjs';
import duration from 'dayjs/plugin/duration';
import relativeTime from 'dayjs/plugin/relativeTime';
import React from 'react';

import {LiveDataForNode} from '../asset-graph/Utils';
Expand All @@ -9,6 +11,9 @@ import {humanCronString} from '../schedules/humanCronString';
const STALE_OVERDUE_MSG = `A materialization incorporating more recent upstream data is overdue.`;
const STALE_UNMATERIALIZED_MSG = `This asset has never been materialized.`;

dayjs.extend(duration);
dayjs.extend(relativeTime);

type LiveDataWithMinutesLate = LiveDataForNode & {
freshnessInfo: NonNullable<LiveDataForNode['freshnessInfo']> & {currentMinutesLate: number};
};
Expand All @@ -20,7 +25,7 @@ export function isAssetLate(liveData?: LiveDataForNode): liveData is LiveDataWit
}

export const humanizedLateString = (minLate: number) =>
`${moment.duration(minLate, 'minute').humanize(false, {m: 120, h: 48})} late`;
`${dayjs.duration(minLate, 'minutes').humanize(false)} late`;

export const CurrentMinutesLateTag: React.FC<{
liveData: LiveDataForNode;
Expand Down
8 changes: 8 additions & 0 deletions js_modules/dagit/packages/eslint-config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ module.exports = {
name: 'lodash',
message: 'Please import specific lodash modules, e.g. `lodash/throttle`.',
},
{
name: 'moment',
message: 'Please use native Intl APIs for date/time, or dayjs if necessary.',
},
{
name: 'moment-timezone',
message: 'Please use native Intl APIs for date/time, or dayjs if necessary.',
},
{
name: 'styled-components',
message: 'Please import from `styled-components/macro`.',
Expand Down

0 comments on commit 5d83868

Please sign in to comment.