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.

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

TypeScript Rollout Tier 9 - Datepicker #379

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

kikuomax
Copy link
Collaborator

Related issues:

Proposed Changes

  • Migration of datepicker

Rewrites the datepicker components in the `src/components/datepicker`
folder in TypeScript.

Adds a new file `types.ts` which defines common types shared among the
datepicker components.

Makes some props explicitly allow `null` to suppress type errors at
lines where `null` is assigned to them.

Applies the `valueOf` method to `Date` when it is passed to `isNaN`
because `isNaN` allows only a number. It should not matter to the
decision. In some cases where the `Date` parameter may be `undefined`,
we have to be careful not to apply `valueOf` to `undefined` and replace
`undefined` with `NaN`.

Replaces some `&&` chaining with ternary conditional operators (`?:`),
where a fallback value should be `undefined` rather than a boolean
(`false`).

Overuses non-null assertion operators (`!`) to suppress type errors
while keeping the original behavior. I decided not to use optional
chaining operators (`?`) unless unavoidable, because they change the
pass of the execution.

Overuses type casting (`as`) to suppress type errors while keeping the
original behavior. I did not take approaches that were more
sophisticated but involving logic changes, since we decided to change
the code, especially its logics, as little as possible during the
migration process.

Some trivial changes:
- Wraps the entire component definition in a `defineComponent` call
- All the Buefy components are directly named when they are registered;
  the `name` fields are no longer used. Otherwise type-checking won't
  work.
- Replaces JSDoc-style comments with ordinary comments so that
  `@microsoft/api-extractor` won't pick them up for documentation.
Rewrites the specs for the datepicker components in the
`src/components/datepicker` in TypeScript.

In `Datepicker.spec.js→ts`:
- Hoists the `defaultProps` fixture to the top level so that its type is
  implicitly defined. And makes it a function so that it reflects the
  configuration made by `setOptions`.
- `focusedDate` is no longer injected into `config` via `setOptions` but
  given as an independent constant. Because `focusedDate` is not a
  member of `BuefyConfig`, and `BuefyConfig` is not a container of
  arbitrary values.

In `DatepickerMonth.spec.js→ts`:
- `newDate` checks if the `y`, `m`, and `d` arguments are defined,
  because `Date` does not like them undefined. Creates `Date` without
  arguments if they are omitted.
- `focusedDate` is no longer injected into `config` via `setOptions` but
  given as an independent constant. See the comments for
  `Datepicker.spec` for why.
- Gives a `Date` instead of a number to the following methods:
  - `emitChosenDate`
  - `updateSelectedDate`

  Allowing a number to these methods does not make sense.

In `DatepickerTable.spec.js→ts`:
- Hoists the `defaultProps` fixture to the top level so that its type
  can be implicitly defined
- `focusedDate` is no longer injected into `config` via `setOptions` but
  given as an independentconstant. See the comments for
  `Datepicker.spec` for why.

In `DatepickerTableRow.spec.js→ts`:
- `newDate` checks if the `y`, `m`, and `d` arguments are defined,
  because `Date` does not like them undefined. Create `Date` without
  arguments if they are omitted.
- Gives a `Date` instead of a number to the following methods:
  - `emitChosenDate`
  - `updateSelectedDate`

  Allowing a number to these methods does not make sense.
- `$emit` of the injected fake `$datepicker` explicitly receives
  parameters to suppress the type errors regarding `arguments`.

Common in the spec files:
- `newDate` checks if the `y`, `m`, and `d` arguments are defined.

In the `__snapshots__` subfolder, replaces the spec snapshots produced
by Jest with those by Vitest:
- `Datepicker.spec.js.snap` → `Datepicker.spec.ts.snap`
- `DatepickerMonth.spec.js.snap` → `DatepickerMonth.spec.ts.snap`
- `DatepickerTable.spec.js.snap` → `DatepickerTable.spec.ts.snap`
- `DatepickerTableRow.spec.js.snap` → `DatepickerTableRow.spec.ts.snap`

Migrates from Jest to Vitest:
- Imports the building blocks for the spec from the `vitest` package
- Replaces `jest` with `vi`
`rollup.config.mjs` removes "datepicker" from `JS_COMPONENTS`.
Rewrites the documentation for the datepicker components in the
`src/pages/components/datepicker` folder in TypeScript. All the changes
are straightforward.

Here is a TypeScript migration tip:
- Explicitly import and register components so that they are type
  checked. No type-checking is performed for globally registered
  components.
@kikuomax kikuomax requested a review from wesdevpro January 13, 2025 04:47
@kikuomax kikuomax merged commit 2fb5c9f into ntohq:dev Jan 13, 2025
18 checks passed
@kikuomax kikuomax deleted the ts-rollout-tier-9-datepicker branch January 13, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeScript Rollout Tier 9
2 participants