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 5 - Loading #338

Merged
merged 5 commits into from
Jan 5, 2025

Conversation

kikuomax
Copy link
Collaborator

@kikuomax kikuomax commented Jan 2, 2025

Related issues:

Proposed Changes

  • Migration of loading and related docs
    • packages/buefy-next/rollup.config.mjs, and packages/buefy-next/src/utils/vue-augmentation.ts may cause conflicts after merging other PRs in this tier.

Rewrites the Loading component in `src/components/loading` in
TypeScript:
- In `Loading.vue`
  - Assumes the `container` prop as `HTMLElement` while it actually
    accepts `Object` and `Function` too. According to the past commit
    0b6f62a, `Object` and `Function`
    are necessary in the server-side-rendering (SSR) context, because
    there is no `HTMLElement` defined in SSR. I am not confident about
    why `Function` is necessary in SSR but I guess `Object` may be just
    a constructor `Function` in some context.

  - Introduces a new type `LoadingProps` which is used to define the
    parameters necessary to programmatically open `Loading`.

  - The `close` method takes arbitrary parameters and expresses them
    with the spread syntax of the rest parameters instead of the
    `argument` keyword to suppress a type error.

  - Trivial changes:
    - Adds `lang="ts"` to the `<script>` section
    - Wraps the entire component definition with a `defineComponent`
      call
    - Gives minimal types
    - Makes JSDoc comments oridinary ones so that
      `@microsoft/api-extractor` won't pick them up

- In `index.js`:
  - Introduces a new type `LoadingOpenParams` which represents the
    parameters that the `open` method accepts. It is essentially
    `LoadingProps` except for `programmatic` which has to be `true`.

  - Stops using `merge` to add `programmatic: true` to the parameters
    given to the `open` method, because it complicated typing. Instead,
    `programmatic: true` is directly embedded in the props given to
    `createElement`.

  - Introduces a new internal type `LoadingProgrammaticInstance` which
    represents a return value of the `open` method. It provides the
    `close` method.

  - Trivial changes:
    - Replaces the extension: ".js" → ".ts"
    - Gives minimal types
Adds `loading` to the Vue global property `$buefy`.
Rewrites `src/components/loading/Loading.spec.js` in TypeScript:
- Introduces a new type `LoadingProgrammaticInstance` which represents a
  return value from the `open` method. It indirectly derives the type
  with `ReturnType`, because I decided not to export the one defined in
  `src/components/loading/index.ts`

- Trivial changes:
  - Replaces the extension: ".js" → ".ts"
  - Imports spec building blocks from `vitest`
  - Replaces `jest` with `vi`
  - Gives minimal types

Replaces the spec snapshot in the `src/components/loading/__snapshots__`
folder with the one generated by vitest: `Loading.spec.js.snap` →
`Loading.spec.ts.snap`
`rollup.config.mjs` removes "loading" from `JS_COMPONENTS`.
Rewrites the documentation for `Loading` in
the `src/pages/components/loading` 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 2, 2025 07:27
@kikuomax kikuomax merged commit 9efea19 into ntohq:dev Jan 5, 2025
18 checks passed
@kikuomax kikuomax deleted the ts-rollout-tier-5-loading branch January 5, 2025 00:57
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.

2 participants