From a6fa02aaec1ec5cba1fa010cf6910f65a965eaf7 Mon Sep 17 00:00:00 2001 From: Tanmay Laud <31733620+tanmaylaud@users.noreply.github.com> Date: Fri, 7 Aug 2020 22:00:17 +0530 Subject: [PATCH] chore: Migrate Timer component from jsx to tsx (#10455) * migrated LanguagePicker.jsx to tsx * Migrated Menu.jsx to tsx * migrated MenuObject.jsx to tsx * migrated NewMenu.jsx to tsx * Migrated UserMenu.jsx to tsx * removed unnecessary export from UserMenu * added language definition in LanguagePicker * removed unnecessary exports from Menu.tsx * used typeof guard for childs * changed LanguageProps to Languages * removed unnecessary type casting * fixed linting errors * migrated Checkbox to tsx * Migrated Timer component to tsx * fixed linting errors * fixed test cases * removed unused import in timer spec * reverting changes * renamed and then modified Timer * changes for review comments * fixed incorrect clear * using stopTimer in stopwatch * fixed lint issues * added explicit timer cleanup * fixed lint issue * fixed memory leak * renamed Timer * added changes after git mv --- .../spec/javascripts/sqllab/Timer_spec.jsx | 26 +----- superset-frontend/src/components/Timer.jsx | 89 ------------------- superset-frontend/src/components/Timer.tsx | 80 +++++++++++++++++ 3 files changed, 83 insertions(+), 112 deletions(-) delete mode 100644 superset-frontend/src/components/Timer.jsx create mode 100644 superset-frontend/src/components/Timer.tsx diff --git a/superset-frontend/spec/javascripts/sqllab/Timer_spec.jsx b/superset-frontend/spec/javascripts/sqllab/Timer_spec.jsx index 49efac73d8..19481679e3 100644 --- a/superset-frontend/spec/javascripts/sqllab/Timer_spec.jsx +++ b/superset-frontend/spec/javascripts/sqllab/Timer_spec.jsx @@ -18,8 +18,6 @@ */ import React from 'react'; import { styledMount as mount } from 'spec/helpers/theming'; -import sinon from 'sinon'; - import Timer from 'src/components/Timer'; import { now } from 'src/modules/dates'; @@ -40,28 +38,10 @@ describe('Timer', () => { expect(React.isValidElement()).toBe(true); }); - it('componentWillMount starts timer after 30ms and sets state.clockStr', async () => { - expect(wrapper.state().clockStr).toBe(''); + it('useEffect starts timer after 30ms and sets state of clockStr', async () => { + expect(wrapper.find('span').text()).toBe(''); await new Promise(r => setTimeout(r, 35)); - expect(wrapper.state().clockStr).not.toBe(''); - }); - - it('calls startTimer on mount', () => { - // Timer is already mounted in beforeEach - wrapper.unmount(); - const startTimerSpy = sinon.spy(Timer.prototype, 'startTimer'); - wrapper.mount(); - // Timer is started once in willUnmount and a second timer in render - // TODO: Questionable whether this is necessary. - expect(startTimerSpy.callCount).toBe(2); - startTimerSpy.restore(); - }); - - it('calls stopTimer on unmount', () => { - const stopTimerSpy = sinon.spy(Timer.prototype, 'stopTimer'); - wrapper.unmount(); - expect(stopTimerSpy.callCount).toBe(1); - stopTimerSpy.restore(); + expect(wrapper.find('span').text()).not.toBe(''); }); it('renders a span with the correct class', () => { diff --git a/superset-frontend/src/components/Timer.jsx b/superset-frontend/src/components/Timer.jsx deleted file mode 100644 index 7fe83a0a65..0000000000 --- a/superset-frontend/src/components/Timer.jsx +++ /dev/null @@ -1,89 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import React from 'react'; -import PropTypes from 'prop-types'; - -import Label from 'src/components/Label'; -import { now, fDuration } from '../modules/dates'; - -const propTypes = { - endTime: PropTypes.number, - isRunning: PropTypes.bool.isRequired, - startTime: PropTypes.number, - status: PropTypes.string, -}; - -const defaultProps = { - endTime: null, - startTime: null, - status: 'success', -}; - -export default class Timer extends React.PureComponent { - constructor(props) { - super(props); - this.state = { - clockStr: '', - }; - this.stopwatch = this.stopwatch.bind(this); - } - UNSAFE_componentWillMount() { - this.startTimer(); - } - componentWillUnmount() { - this.stopTimer(); - } - startTimer() { - if (!this.timer) { - this.timer = setInterval(this.stopwatch, 30); - } - } - stopTimer() { - this.timer = clearInterval(this.timer); - } - stopwatch() { - if (this.props && this.props.startTime) { - const endDttm = this.props.endTime || now(); - if (this.props.startTime < endDttm) { - const clockStr = fDuration(this.props.startTime, endDttm); - this.setState({ clockStr }); - } - if (!this.props.isRunning) { - this.stopTimer(); - } - } - } - render() { - if (this.props && this.props.isRunning) { - this.startTimer(); - } - let timerSpan = null; - if (this.props) { - timerSpan = ( - - ); - } - return timerSpan; - } -} - -Timer.propTypes = propTypes; -Timer.defaultProps = defaultProps; diff --git a/superset-frontend/src/components/Timer.tsx b/superset-frontend/src/components/Timer.tsx new file mode 100644 index 0000000000..ece45e0940 --- /dev/null +++ b/superset-frontend/src/components/Timer.tsx @@ -0,0 +1,80 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React, { useEffect, useState } from 'react'; +import { Label } from 'react-bootstrap'; + +import { now, fDuration } from '../modules/dates'; + +interface TimerProps { + endTime?: number; + isRunning: boolean; + startTime?: number; + status?: string; +} + +export default function Timer({ + endTime, + isRunning, + startTime, + status = 'success', +}: TimerProps) { + const [clockStr, setClockStr] = useState(''); + const [timer, setTimer] = useState(); + + const stopTimer = () => { + if (timer) { + clearInterval(timer); + setTimer(undefined); + } + }; + + const stopwatch = () => { + if (startTime) { + const endDttm = endTime || now(); + if (startTime < endDttm) { + setClockStr(fDuration(startTime, endDttm)); + } + if (!isRunning) { + stopTimer(); + } + } + }; + + const startTimer = () => { + setTimer(setInterval(stopwatch, 30)); + }; + + useEffect(() => { + if (isRunning) { + startTimer(); + } + }, [isRunning]); + + useEffect(() => { + return () => { + stopTimer(); + }; + }); + + return ( + + ); +}