Skip to content

Commit

Permalink
TypeScript Rollout Tier 8 - TimepickerMixin (#372)
Browse files Browse the repository at this point in the history
* feat(lib): rewrite TimepickerMixin in TS

Rewrites `src/utils/TimepickerMixin.js` in TypeScript. Also includes a
fix of an obvious issue that the obsolete `value` prop was used.

Introduces a new interface `ITimepickerMixin` which extracts the props,
and computed values of `TimepickerMixin` as a super type. While
`TimepickerMixin` contains itself in its type definition, we can avoid
type recursive by using `ITimepickerMixin` instead; see `TimeFormatter`,
and `TimeParser`.

Introduces new function types:
- `TimeCreator`: type of the `timeCreator` prop
- `TimeFormatter`: type of the `timeFormatter` prop
- `TimeParser`: type of the `timeParser` prop

Introduces a new type `HourFormat` which is a union of the constants
`HOUR_FORMAT_24` and `HOUR_FORMAT_12` and represents the type of the
`hourFormat` prop.

The following methods of `src/utils/TimepickerMixin.ts` accept `number`
in addition to `string`:
- `onHoursChange`
- `onMinutesChange`
- `onSecondsChange`

The `minTime` and `maxTime` props of `TimepickerMixin` allow `null` so
that they meet the requirements of `Datetimepicker`.

Some tricky changes:
- Applies the `valueOf` method to the `Date` argument before passing it
  to `isNaN`, because `isNaN` does not accept a `Date` type in
  TypeScript. As far as I tested on Node v18, `Date.valueOf` returned
  `NaN` when `isNaN` was expected to return `true`.
- Performs an extra concatenation with an empty string (`+ ''`) is on
  some arguments given to `parseInt` to make sure they are strings.

Maybe too optimistic assumptions:
- While the `Intl.DateTimeFormat.resolvedOptions` returns
  `ResolvedDateTimeFormatOptions`, it is casted as
  `DateTimeFormatOptions` without checks
- Supposes the `input` ref to refer to an `Input` component instance
- Supposes the `dropdown` ref to refer to a `Dropdown` component
  instance
- Supposes the `target` field of the `event` parameter of the
  `onChangeNativePicker` method to be an `HTMLInputElement`

The `modelValue` prop allows `null`.

The following changes are trivial but I want to leave reasons for them:
- Replaces JSDoc comments with ordinary ones so that
  `@microsoft/api-extractor` won't pick them up for documentation
- Overuses non-null assertions (!) to keep the original behavior

* test(lib): rewrite spec for TimepickerMixin in TS

Rewrites `src/utils/TimepickerMixin.spec.js` in TypeScript.

Passes "0" to the `onSecondsChange` method, because it now requires a
string.

Moves the definition of the test component before `wrapper` so that
`wrapper` can be properly typed with `VueWrapper`.

Trivial changes:
- Replaces the extension: ".js" → ".ts"
- Imports the spec building blocks from the `vitest` package
- Replaces `jest` with `vi`
  • Loading branch information
kikuomax authored Jan 12, 2025
1 parent 5eda02f commit 0b188ca
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 82 deletions.
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { defineComponent } from 'vue'
import { shallowMount } from '@vue/test-utils'
import type { VueWrapper } from '@vue/test-utils'
import TimepickerMixin from '@utils/TimepickerMixin'

const AM = 'AM'
const PM = 'PM'
let wrapper

const component = defineComponent({
mixins: [TimepickerMixin],
template: '<div class="b-component"></div>'
})

let wrapper: VueWrapper<InstanceType<typeof component>>

describe('TimepickerMixin', () => {
HTMLElement.prototype.insertAdjacentElement = jest.fn()
HTMLElement.prototype.insertAdjacentElement = vi.fn()
beforeEach(() => {
const component = {
template: '<div class="b-component"></div>',
mixins: [TimepickerMixin]
}
wrapper = shallowMount(component, {
attachTo: document.body
})
Expand All @@ -30,8 +35,8 @@ describe('TimepickerMixin', () => {
})

it('should call updateDateSelected on onSecondsChange', async () => {
wrapper.vm.updateDateSelected = jest.fn()
wrapper.vm.onSecondsChange()
wrapper.vm.updateDateSelected = vi.fn()
wrapper.vm.onSecondsChange('0')
await wrapper.vm.$nextTick()
expect(wrapper.vm.updateDateSelected).toHaveBeenCalledTimes(1)
})
Expand All @@ -55,14 +60,14 @@ describe('TimepickerMixin', () => {
})

it('should call onFocus on handleOnFocus', async () => {
wrapper.vm.onFocus = jest.fn()
wrapper.vm.onFocus = vi.fn()
wrapper.vm.handleOnFocus()
await wrapper.vm.$nextTick()
expect(wrapper.vm.onFocus).toHaveBeenCalledTimes(1)
})

it('should call toggle on close', async () => {
wrapper.vm.toggle = jest.fn()
wrapper.vm.toggle = vi.fn()
wrapper.vm.close()
await wrapper.vm.$nextTick()
expect(wrapper.vm.toggle).toHaveBeenCalledTimes(1)
Expand Down
Loading

0 comments on commit 0b188ca

Please sign in to comment.