forked from buefy/buefy
-
Notifications
You must be signed in to change notification settings - Fork 11
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 1 #319
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Installs `rollup-plugin-esbuild` that adds TypeScript support to Rollup. This package supersedes `@rollup/plugin-typescript`. Replaces `rollup-plugin-vue` with `@vitejs/plugin-vue` because it is no longer maintained. `@vitejs/plugin-vue` supersedes it. There were some possible combinations of plugins for TypeScript and Vue, though, my preliminary experiments showed that the combination of `rollup-plugin-esbuild` and `@vitejs/plugin-vue` looks promising. Installs a couple of additional packages: - `vue-tsc`: to facilitate type check - `@vue/tsconfig`: to bootstrap TypeScript config - Introduces a new script `type-check` that runs `vue-tsc` to perform type check without emitting results.
- Removes `src/types` folder because we will rewrite all the components in TypeScript and type declarations will be generated automatically.
Configures `rollup.config.mjs` so that Rollup can process TypeScript in both .ts and .vue files. Replaces `babel` with `esbuild` that has built-in support for TypeScript. `tsconfig.json` is a basic configuration for TypeScript. It has `allowJs` option `true` since we have bunch of JavaScript files, though, this option shall be removed when we finish migration to TypeScript. No type declaration files (.d.ts) are generated for now. We have to do it in another script. Renames `src/index.js` to `src/index.ts`, because TypeScript requires at least one `ts` file included in the source files.
- `src/index.ts` imports `App` type from `vue`. It reduces one error by the `type-check` script.
- Installs and configures `vitest`. Only `*.spec.ts` files are evaluated.
`type-check` script uses a new TypeScript config `tsconfig.test.json` that includes `*.spec.*` files. `tsconfig.json` adds `*.spec.*` to `exclude` option to exclude `*.spec.ts` files.
Adds a new script `build:dts` that generates and rolls up type declarations. The rolled up type declaration is emitted to a new place `dist/buefy.d.ts`. `build` script also runs `build:dts`. Uses `@microsoft/api-extractor` (`api-extractor`) to roll up type declarations. `api-extractor.json` is the configuration file for `api-extractor`. Introduces a new TypeScript config `tsconfig.types.json` that enables options for generating type declarations while disabling transpilation.
- Configures ESLint to run on TypeScript (*.ts) files. Installs the following packages necessary for TypeScript processing: - @typescript-eslint/eslint-plugin - @typescript-eslint/parser Updates `.eslintrc.js` according to the following instructions: - https://typescript-eslint.io/getting-started#step-2-configuration - https://typescript-eslint.io/linting/troubleshooting/#i-am-running-into-errors-when-parsing-typescript-in-my-vue-files Some ESLint errors are detected due to introduction of the TypeScript rules.
- Changes the entry file of `@ntohq/buefy-next` in the development mode: `index.js` → `index.ts` Adds `.ts` to `resolve.extensions`.
- Adds a new script `type-check` that checks types. `tsconfig.json` is a basic configuration for TypeScript. `allowJs` option in `tsconfig.json` shall be removed after TypeScript migration completes.
Fixes and works around type errors in `main.ts`: - Removes `vueApp.config.productionTip` because it no longer exists in Vue 3. See https://v3-migration.vuejs.org/breaking-changes/global-api.html#config-productiontip-removed - Works around type errors related to tiny-emitter - Augments Vue component instance so that `$http` and `$eventHub` are available in every component.
- `build` script runs both `type-check` and `vite build` in parallel. Introduces a new script `build-only` that runs only `vite build`. Installs `npm-run-all2` to parallelly runs scripts.
- To prevent version discrepancies, hoists the following dependencies used in both `buefy-next` and `docs` packages: - `vue` - `vite` - `@vitejs/plugin-vue` - `vue-tsc` - `@vue/tsconfig` - `sass`
Installs missing type definitions that `packages/docs` depends on: - `@types/lodash` - `@types/scrollreveal` - `@types/cleave.js`: Although the installed version of `cleave.js` is 1.0.1, `@types/cleave.js` v1.4.12 is installed. Because older versions did not correctly declare its types. It should not harm as long as the actual usage of `cleave.js` does not change. - `@types/sortablejs`: I have no idea about which version of `@types/sortablejs` I should install, though, it should not matter as long as the code actually using `sortablejs` does not change, and type-checking passes. - Applies npm audit suggestions
Introduces TypeScript project references so that `packages/docs` can import files and declaration files from `packages/buefy-next`. The new `tsconfig.json` in the root folder defines which tsconfig files are included in the project. As we focus on type declarations of `packages/buefy-next`, only `tsconfig.types.json` is included from `packages/buefy-next` into the project. `packages/docs` introduces a new `tsconfig.types.json` file that is configured not to import types from `packages/buefy-next/dist` but to directly include `package/buefy-next/src` during type check. `packages/docs/tsconfig.types.json` is also included in the project. These tsconfig files must have "composite" and "declaration" `true`. Please refer to TypeScript documentation for more details about "Project Reference". https://www.typescriptlang.org/docs/handbook/project-references.html `.gitignore` ignores `*.tsbuildinfo` files that are generated if project references are enabled. In `packages/docs`, `type-check` now uses `tsconfig.types.json` as the TypeScript configuration to directly include `packages/buefy-next/src`. `build` script calls a new script `build:type-check` that uses `tsconfig.json` as the TypeScript configuration and imports types from `packages/buefy-next/dist` instead. Also fixes the error in `packages/buefy-next/tsconfig.types.json` that "emitDeclarationOnly" was turned on while "noEmit" is `true`. Sets "noEmit" `false`.
- Lets `api-extractor` fail if there is any compiler error, but not fail if there is any TSDoc related warnings even in production; we get a lot of such warnings due to missing release quality tags; i.e., `@alpha`, `@beta`, `@public`, and `@internal`.
Includes `vite/client` in `compilerOptions.types` so that Vite specific qualifiers can be properly typed; e.g., `?raw`. Also includes "jsdom" to suppress `window` related errors. `tsconfig.types.json` extends `tsconfig.json` instead of `@vue/tsconfig/tsconfig.dom.json` to avoid discrepancies between development and production. Adds "@/" as an alias of the `src` folder to `paths` in both `tsconfig.json` and `tsconfig.types.json`.
- Fixes the type error caused at the line where components are installed to the Vue app. `components/index.js` is renamed to `components/index.ts` and exports an array of components as default. `index.ts` extracts components from the default export of `components/index.ts` instead of enumerating all the exported items from `components/index.ts`.
- Exports individual components from `components/index.ts` and `index.ts` so that users can benefit from type checking. For now, users have to import individual Buefy components they use to check types of the components; type checking won't work on components globally installed by `App.use(Buefy)`. `components/index.ts` used to export only install functions of components.
- `.gitignore` ignores the `temp-dts` folder that temporarily keeps type declarations.
Introduces a new file `utils/vue-augmentation.ts` that declares the module augmentation for `Vue` so that Vue components can access the Buefy global API through `this.$buefy`. It augments `ComponentCustomProperties` to include `$buefy`. `$buefy` only has `globalNoticeInterval` and other fields typed `any` for now, but must be refined in the future. This file will result in `vue-augmentation.d.ts` after TypeScript processing, which must be appended to `buefy.d.ts` because `api-extractor` won't include the module augmentation in the rolled up output. A new script `build/augment-dts.mjs` does this job, and `build:dts` script runs `build/augment-dts.mjs` after `api-extractor` outputs `buefy.d.ts`. `build/augment-dts.mjs` excludes lines starting with `import` from `src/utils/vue-augmentation.d.ts` before appending it to the declaration file body. Because the imported paths should be already in the declaration file body and relative imports will spoil those types. `index.ts` includes `utils/vue-augmentation.ts` to activate the module augmentation.
Replaces `module` field of `package.json`: `dist/esm/index.js` → `dist/buefy.esm.js`.
- Fixes the error that `build/vetur.js` failed to load the Tag API from `.js` file because the Tag API was replaced with the `.ts` file. Tries to read `.ts` file first, and then falls back to `.js` file. This workaround will become unnecessary when we complete TypeScript migration.
Works around the type error in `ExProgrammatically` example of `Datepicker`. Adds `<script>` section without the language prop to make it a JavaScript component. This has to be reverted after we complete TypeScript migration.
- Adds test cases for `matchWithGroups` defined in `utils/helpers.js`. As rewriting `matchWithGroups` in TypeScript will involve some changes in the flow, we should define tests before rewriting it.
- Adds tests for `toCssWidth` defined in `utils/helpers.js`. As rewriting `toCssWidth` in TypeScript will involve a behavioral change to it, we should define tests for it. The change is how `NaN` is handled: - before: `NaN` is returned as `NaN` (number) - after: `NaN` is returned as "NaN" (string) By the above change, we can declare the return value as `string` or `null`. Passing `NaN` to `toCssWidth` is erroneous anyway. Also updates `utils/helpers.js` to pass the tests.
- This commit is just for the record of the name change.
Rewrites `utils/helper.js` in TypeScript: - Augments `App` in `@vue/runtime-core` so that `__VUE_I18N_SYMBOL_` introduced by `vue-i18n` may be optionally handled. - Adds `@internal` directives in TSDoc comments. By adding `@internal` directives, `api-extractor` somehow failed with the following error: ERROR: Internal Error: Unable to analyze the export "getValueByPath" in /buefy/packages/buefy-next/temp-dts/utils/helpers.d.ts You have encountered a software defect. Please consider reporting the issue to the maintainers of this application. If I removed the `@internal` scope from `getValueByPath`, `api-extractor` started to fail with a similar error around `mod`. The error disappered by removing the `@internal` scope from `getValueByPath` and moving `export` statements to the function definitions of `mod`, `bound`, and `hasFlag`. - `toCssWidth` tweaks `isNaN(width)` as `isNaN(+width)` to avoid the type error caused because `isNaN` does not accept a string. - `matchWithGroups` makes sure `pattern` contains group names, otherwise throws `RangeError`. It also removes a redundant check of group names that should never become `true`. - Introduces a new utility type `DeepPartial` to properly type `mergeFn`. - `isCustomElement` picks only the `$root` field of `ComponentPublicInstance`. We may face type errors if we use `ComponentPublicInstance` with default type parameters. - Introduces a new interface `MultiColumnSortPriority` which represents the element type of the `sortingPriority` parameter of `multiColumnSort`. And replaces the type of `sortingPriority`: `string[]` → `MultiColumnSortPriority[]`. - Introduces a new interface `TranslateTouchAsDragEventOptions` which represents the `options parameter of the `translateTouchAsDragEvent` - Introduces a new type utility `ExtractComponentProps` that extracts the props type from a component constructor. - Introduces a new helper type `ExtractComponentData` that extracts the data type from the constructor of a component. It may help us avoiding use of `any` when we `mount` or `shallowMount` a component that has mixins in tests using Vue test utils. As far as I tested, `mount` and `shallowMount` do not respect data fields in mixins. - `isVueComponent` narrows the parameter `c` to `ComponentPublicInstance`. - `isDefined` makes sure that the argument is not `undefined` by type narrowing
- Rewrites tests for `utils/helpers.ts` in TypeScript: `utils/helpers.spec.js` → `.ts`: - Imports necessary functions and object from `vitest` - Replaces `jest.fn` with `vi.fn` - Gives types to the comparison function given to `indexOf` - Specifies type parameters to `merge` calls - Adds a test case for `merge`, which needs `DeepPartial` utility type to pass type check
The `unit` npm script now runs `vitest`. It no longer updates snapshots. If you need to update snapshots, use a new npm script `unit:update` or the `test` script.
Increments the minor version, because I think the TypeScript migration is drastic enough and includes a few breaking changes.
Introduces a new GitHub Actions workflow `verify.yml` which performs linting, type-checking, and unit testing when a commit is pushed or a pull request is made to a specific branch. It will replace `unit_testing.yml` when the TypeScript migration is complete. Although the `migrate-to-typescript` is included in the trigger branches, it should be removed when the TypeScript migration is complete.
The `verify.yml` workflow introduces a new job `verify-docs` which is triggered when commits are pushed to, or a PR is made to the `dev` or `main` branch, and runs the `lint` and `type-check` scripts on the `docs` package.
The `verify.yml` workflow is no longer triggered by the `migrate-to-typescript` branch.
The `verify.yml` workflow skips linting. Revert this at the end of the TypeScript migration.
Removes the `unit_testing.yml` workflow, because `verify.yml` does the jobs instead.
Updates the development guideline in `.github/CONTRIBUTING.md`. Remarkable changes: - Gives separate instructions for the library (`packages/buefy-next`) and the documentation (`packages/docs`) - Introduces type-checking command - Includes testing and linting in the points to be considered before a PR submission - Increases the minimum Node.js version to 16 which is required by some of `devDependencies`. See the commit c9aad5b. - Removes leading `$`s from the code snippet so that readers can replay the commands on their shell by copy&paste without editing them
Updates `rollup.config.mjs` so that it can bundle files that have already migrated to TypeScript. `rollup.config.mjs` should be incrementally updated when it comes to being merged to `dev` branch; every time you rewrite a component in TypeScript, you have to remove the component category from `JS_COMPONENTS`. So that we can merge PRs in arbitrary order to rollout TypeScript migration of components one by one, I decided to list .js components rather than .ts components. Because I thought removing lines for `JS_COPONENTS` are less ambiguous compared to adding lines to `TS_COMPONENTS` in terms of ordering.
wesdevpro
approved these changes
Dec 29, 2024
LGTM 🚀 |
We got the following error during the
This discussion comment might help. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issues:
Proposed Changes
package.json
npm run type-check
: Performs type-checkingnpm run build:dts
: Builds the type declaration (dist/buefy.d.ts
) fileJS_COMPONENTS
lists all the component groups that are not yet rewritten in TypeScript. Every time a component group migrates to TypeScript, it is removed fromJS_COMPONENTS
. The array contains all the component groups in this PR.@microsoft/api-extractor
for the type declaration (dist/buefy.d.ts
) file generation.github/workflows/unit_testing.yml
with.github/workflows/verify.yml
which performs linting, type-checking, and unit-testing, and also does linting, and type-checking of the docs. However, lint errors are tolerated until the TypeScript migration is complete.src/types
because they are no longer necessarypackages/buefy-next/src/index.js
topackages/buefy-next/src/index.ts
becausetsc
requires at least one.ts
file. Also renamespackages/docs/src/main.js
topackages/docs/src/main.ts
for the same reason..js
files, rewritespackages/buefy-next/src/utils/helpers.js
in TypeScript. I chose this file because:packages/buefy-next/src/utils/vue-augmentation.ts
which declares the$buefy
Vue global property. A new build scriptpackages/buefy-next/build/augment-dts.mjs
transforms thevue-augmentation.ts
file and appends it to the type declaration (dist/buefy.d.ts
) file.