From f0fa2c82dce0cb507b892645e27a9fafd8ab87e0 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Mon, 19 Feb 2024 05:37:36 -0500 Subject: [PATCH] Replace `ts-loader` with `@babel/preset-typescript` and `fork-ts-checker-webpack-plugin` (#35713) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Replace `ts-loader` with `@babel/preset-typescript` and `fork-ts-checker-webpack-plugin` We've decided (pdWQjU-FO-p2) to prefer `@babel/preset-typescript` over `ts-loader` for dealing with TypeScript in the context of Webpack. For generating the definition files, we're generally going to use `fork-ts-checker-webpack-plugin` (although `tsc --emitDeclarationOnly` is also an option). Also, as cleanup, these packages are switched to `moduleResolution:nodenext` to ensure the `.d.ts` files are usable by consumers using that rather than `bundler`. Of the packages affected by this: * image-guide apparently just had a dep on ts-loader but doesn't use it. * videopress-core has no changes in the build artifacts. * boost-score-api, react-data-sync-client, and svelte-data-sync-client all wind up with smaller bundles (by 14K–19K) now that Babel decides what needs transpiling based on the browserslist config, so they use native classes, generators, promises, and array-spread operators now. This PR also adds a linting check to warn against re-introduction of ts-loader, and mentions this in the relevant doc. * Fix missing dependency * changelog * Revert 7aab051 and f94d463 --------- Co-authored-by: Mark George --- .github/files/lint-project-structure.sh | 7 +++++ docs/coding-guidelines.md | 2 ++ pnpm-lock.yaml | 30 ------------------- .../changelog/fix-remove-use-of-ts-loader | 4 +++ .../js-packages/boost-score-api/package.json | 1 - .../js-packages/boost-score-api/tsconfig.json | 3 +- .../boost-score-api/webpack.config.cjs | 16 ++++++---- .../changelog/fix-remove-use-of-ts-loader | 5 ++++ projects/js-packages/image-guide/package.json | 1 - .../changelog/fix-remove-use-of-ts-loader | 4 +++ .../react-data-sync-client/package.json | 1 - .../react-data-sync-client/tsconfig.json | 6 ++-- .../react-data-sync-client/webpack.config.cjs | 16 ++++++---- .../changelog/fix-remove-use-of-ts-loader | 4 +++ .../changelog/fix-remove-use-of-ts-loader#2 | 4 +++ .../svelte-data-sync-client/package.json | 3 +- .../svelte-data-sync-client/src/DataSync.ts | 6 ++-- .../src/SyncedStore.ts | 6 ++-- .../svelte-data-sync-client/src/index.ts | 6 ++-- .../src/initializeClient.ts | 4 +-- .../svelte-data-sync-client/tsconfig.json | 6 ++-- .../webpack.config.cjs | 16 ++++++---- .../changelog/fix-remove-use-of-ts-loader | 4 +++ .../js-packages/videopress-core/package.json | 1 - .../js-packages/videopress-core/tsconfig.json | 6 ++-- .../videopress-core/webpack.config.cjs | 16 ++++++---- 26 files changed, 96 insertions(+), 82 deletions(-) create mode 100644 projects/js-packages/boost-score-api/changelog/fix-remove-use-of-ts-loader create mode 100644 projects/js-packages/image-guide/changelog/fix-remove-use-of-ts-loader create mode 100644 projects/js-packages/react-data-sync-client/changelog/fix-remove-use-of-ts-loader create mode 100644 projects/js-packages/svelte-data-sync-client/changelog/fix-remove-use-of-ts-loader create mode 100644 projects/js-packages/svelte-data-sync-client/changelog/fix-remove-use-of-ts-loader#2 create mode 100644 projects/js-packages/videopress-core/changelog/fix-remove-use-of-ts-loader diff --git a/.github/files/lint-project-structure.sh b/.github/files/lint-project-structure.sh index 9480f3aec9add..f55284d8a7094 100755 --- a/.github/files/lint-project-structure.sh +++ b/.github/files/lint-project-structure.sh @@ -188,6 +188,13 @@ for PROJECT in projects/*/*; do echo "::error file=$PROJECT/tsconfig.json::The project should have either jsconfig.json or tsconfig.json, not both. Keep tsconfig if the project uses TypeScript, or jsconfig if the project is JS-only." fi + # - We want to use @babel/preset-typescript (and fork-ts-checker-webpack-plugin or tsc for definition files) rather than ts-loader. + if [[ -e "$PROJECT/package.json" ]] && jq -e '.dependencies["ts-loader"] // .devDependencies["ts-loader"] // .optionalDependencies["ts-loader"]' "$PROJECT/package.json" >/dev/null; then + EXIT=1 + LINE=$(jq --stream -r 'if length == 1 then .[0][:-1] else .[0] end | if . == ["dependencies","ts-loader"] or . == ["devDependencies","ts-loader"] or . == ["optionalDependencies","ts-loader"] then ",line=\( input_line_number )" else empty end' "$PROJECT/package.json" | head -1) + echo "::error file=$PROJECT/package.json${LINE}::For consistency we've settled on using \`@babel/preset-typescript\` (and \`fork-ts-checker-webpack-plugin\` or \`tsc\` for definition files) rather than \`ts-loader\`. Please switch to that." + fi + # - composer.json must exist. if [[ ! -e "$PROJECT/composer.json" ]]; then EXIT=1 diff --git a/docs/coding-guidelines.md b/docs/coding-guidelines.md index 422e2197d5f7a..9448cc2e59800 100644 --- a/docs/coding-guidelines.md +++ b/docs/coding-guidelines.md @@ -97,6 +97,8 @@ For more information on how to use `$$next-version$$`, please see the [packages - Use Gutenberg's [@wordpress/i18n](https://www.npmjs.com/package/@wordpress/i18n) package. - Use an appropriate unique text domain in your JS code. - Make use of [@automattic/babel-plugin-replace-textdomain](https://www.npmjs.com/package/@automattic/babel-plugin-replace-textdomain) when bundling to ensure i18n works in the published plugin. +- When using TypeScript in Webpack, use `@babel/preset-typescript` rather than `ts-loader`. + - To generate `.d.ts` files, either `fork-ts-checker-webpack-plugin` or `tsc --emitDeclarationOnly` may be used. ## Where should my code live? diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4fbcdeb3d1fb3..9ca508b45d87f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -243,9 +243,6 @@ importers: jest-environment-jsdom: specifier: 29.7.0 version: 29.7.0 - ts-loader: - specifier: 9.4.2 - version: 9.4.2(typescript@5.0.4)(webpack@5.76.0) typescript: specifier: 5.0.4 version: 5.0.4 @@ -697,9 +694,6 @@ importers: svelte-preprocess: specifier: 5.0.4 version: 5.0.4(@babel/core@7.23.5)(postcss@8.4.31)(sass@1.64.1)(svelte@3.58.0)(typescript@5.0.4) - ts-loader: - specifier: 9.4.2 - version: 9.4.2(typescript@5.0.4)(webpack@5.76.0) tslib: specifier: 2.5.0 version: 2.5.0 @@ -988,9 +982,6 @@ importers: react: specifier: 18.2.0 version: 18.2.0 - ts-loader: - specifier: 9.4.2 - version: 9.4.2(typescript@5.0.4)(webpack@5.76.0) tslib: specifier: 2.5.0 version: 2.5.0 @@ -1349,9 +1340,6 @@ importers: svelte: specifier: 3.58.0 version: 3.58.0 - ts-loader: - specifier: 9.4.2 - version: 9.4.2(typescript@5.0.4)(webpack@5.76.0) tslib: specifier: 2.5.0 version: 2.5.0 @@ -1385,9 +1373,6 @@ importers: jest: specifier: '*' version: 29.7.0 - ts-loader: - specifier: 9.4.2 - version: 9.4.2(typescript@5.0.4)(webpack@5.76.0) tslib: specifier: 2.5.0 version: 2.5.0 @@ -23417,21 +23402,6 @@ packages: yargs-parser: 21.1.1 dev: true - /ts-loader@9.4.2(typescript@5.0.4)(webpack@5.76.0): - resolution: {integrity: sha512-OmlC4WVmFv5I0PpaxYb+qGeGOdm5giHU7HwDDUjw59emP2UYMHy9fFSDcYgSNoH8sXcj4hGCSEhlDZ9ULeDraA==} - engines: {node: '>=12.0.0'} - peerDependencies: - typescript: '*' - webpack: ^5.0.0 - dependencies: - chalk: 4.1.2 - enhanced-resolve: 5.15.0 - micromatch: 4.0.5 - semver: 7.5.2 - typescript: 5.0.4 - webpack: 5.76.0(webpack-cli@4.9.1) - dev: true - /tsconfig-paths@3.15.0: resolution: {integrity: sha512-2Ac2RgzDe/cn48GvOe3M+o82pEFewD3UPbyoUHHdKasHwJKjds4fLXWf/Ux5kATBKN20oaFGu+jbElp1pos0mg==} dependencies: diff --git a/projects/js-packages/boost-score-api/changelog/fix-remove-use-of-ts-loader b/projects/js-packages/boost-score-api/changelog/fix-remove-use-of-ts-loader new file mode 100644 index 0000000000000..0ef77f6133eed --- /dev/null +++ b/projects/js-packages/boost-score-api/changelog/fix-remove-use-of-ts-loader @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Update build configuration to better match supported target environments. diff --git a/projects/js-packages/boost-score-api/package.json b/projects/js-packages/boost-score-api/package.json index 8983077ca41b6..6b679538c6828 100644 --- a/projects/js-packages/boost-score-api/package.json +++ b/projects/js-packages/boost-score-api/package.json @@ -29,7 +29,6 @@ "eslint": "8.51.0", "jest": "29.7.0", "jest-environment-jsdom": "29.7.0", - "ts-loader": "9.4.2", "typescript": "5.0.4", "webpack": "5.76.0", "webpack-cli": "4.9.1" diff --git a/projects/js-packages/boost-score-api/tsconfig.json b/projects/js-packages/boost-score-api/tsconfig.json index 5b43467da7bea..c7d3db094eba1 100644 --- a/projects/js-packages/boost-score-api/tsconfig.json +++ b/projects/js-packages/boost-score-api/tsconfig.json @@ -8,8 +8,7 @@ "outDir": "./build/", "noEmit": false, "declaration": true, - "module": "es6", - "target": "es5", + "module": "nodenext", "moduleResolution": "nodenext" } } diff --git a/projects/js-packages/boost-score-api/webpack.config.cjs b/projects/js-packages/boost-score-api/webpack.config.cjs index c7cc99bf11a21..50c74010eb311 100644 --- a/projects/js-packages/boost-score-api/webpack.config.cjs +++ b/projects/js-packages/boost-score-api/webpack.config.cjs @@ -8,11 +8,10 @@ module.exports = { module: { strictExportPresence: true, rules: [ - { - test: /\.ts?$/, - use: 'ts-loader', - exclude: /node_modules/, - }, + // Transpile JavaScript and TypeScript + jetpackWebpackConfig.TranspileRule( { + exclude: /node_modules\//, + } ), ], }, optimization: { @@ -30,5 +29,10 @@ module.exports = { type: 'umd', }, }, - plugins: [ ...jetpackWebpackConfig.StandardPlugins() ], + plugins: [ + ...jetpackWebpackConfig.StandardPlugins( { + // Generate `.d.ts` files per tsconfig settings. + ForkTSCheckerPlugin: {}, + } ), + ], }; diff --git a/projects/js-packages/image-guide/changelog/fix-remove-use-of-ts-loader b/projects/js-packages/image-guide/changelog/fix-remove-use-of-ts-loader new file mode 100644 index 0000000000000..b5e40b7e5eb6b --- /dev/null +++ b/projects/js-packages/image-guide/changelog/fix-remove-use-of-ts-loader @@ -0,0 +1,5 @@ +Significance: patch +Type: removed +Comment: Remove unused JS dev dependency. + + diff --git a/projects/js-packages/image-guide/package.json b/projects/js-packages/image-guide/package.json index be82f0331deeb..083bbba28f378 100644 --- a/projects/js-packages/image-guide/package.json +++ b/projects/js-packages/image-guide/package.json @@ -54,7 +54,6 @@ "sass": "1.64.1", "svelte": "3.58.0", "svelte-preprocess": "5.0.4", - "ts-loader": "9.4.2", "tslib": "2.5.0", "typescript": "5.0.4", "webpack": "5.76.0", diff --git a/projects/js-packages/react-data-sync-client/changelog/fix-remove-use-of-ts-loader b/projects/js-packages/react-data-sync-client/changelog/fix-remove-use-of-ts-loader new file mode 100644 index 0000000000000..0ef77f6133eed --- /dev/null +++ b/projects/js-packages/react-data-sync-client/changelog/fix-remove-use-of-ts-loader @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Update build configuration to better match supported target environments. diff --git a/projects/js-packages/react-data-sync-client/package.json b/projects/js-packages/react-data-sync-client/package.json index b633608b716e9..042a0aee19701 100644 --- a/projects/js-packages/react-data-sync-client/package.json +++ b/projects/js-packages/react-data-sync-client/package.json @@ -27,7 +27,6 @@ "@tanstack/react-query": "5.0.5", "jest": "*", "react": "18.2.0", - "ts-loader": "9.4.2", "tslib": "2.5.0", "typescript": "5.0.4", "webpack": "5.76.0", diff --git a/projects/js-packages/react-data-sync-client/tsconfig.json b/projects/js-packages/react-data-sync-client/tsconfig.json index a88a0ca2461c4..f8a9be283f0de 100644 --- a/projects/js-packages/react-data-sync-client/tsconfig.json +++ b/projects/js-packages/react-data-sync-client/tsconfig.json @@ -7,8 +7,8 @@ "forceConsistentCasingInFileNames": true, "outDir": "./build/", "noEmit": false, - "declaration": true, - "module": "es6", - "target": "es5" + "module": "nodenext", + "moduleResolution": "nodenext", + "declaration": true } } diff --git a/projects/js-packages/react-data-sync-client/webpack.config.cjs b/projects/js-packages/react-data-sync-client/webpack.config.cjs index a6c4392f341cd..96edc02fd7c1f 100644 --- a/projects/js-packages/react-data-sync-client/webpack.config.cjs +++ b/projects/js-packages/react-data-sync-client/webpack.config.cjs @@ -10,11 +10,10 @@ module.exports = { module: { strictExportPresence: true, rules: [ - { - test: /\.ts?$/, - use: 'ts-loader', - exclude: /node_modules/, - }, + // Transpile JavaScript and TypeScript + jetpackWebpackConfig.TranspileRule( { + exclude: /node_modules\//, + } ), ], }, optimization: { @@ -32,5 +31,10 @@ module.exports = { type: 'umd', }, }, - plugins: [ ...jetpackWebpackConfig.StandardPlugins() ], + plugins: [ + ...jetpackWebpackConfig.StandardPlugins( { + // Generate `.d.ts` files per tsconfig settings. + ForkTSCheckerPlugin: {}, + } ), + ], }; diff --git a/projects/js-packages/svelte-data-sync-client/changelog/fix-remove-use-of-ts-loader b/projects/js-packages/svelte-data-sync-client/changelog/fix-remove-use-of-ts-loader new file mode 100644 index 0000000000000..a28724b17a14b --- /dev/null +++ b/projects/js-packages/svelte-data-sync-client/changelog/fix-remove-use-of-ts-loader @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Adjust build to work with `tsc` and `moduleResolution:nodenext`. diff --git a/projects/js-packages/svelte-data-sync-client/changelog/fix-remove-use-of-ts-loader#2 b/projects/js-packages/svelte-data-sync-client/changelog/fix-remove-use-of-ts-loader#2 new file mode 100644 index 0000000000000..0ef77f6133eed --- /dev/null +++ b/projects/js-packages/svelte-data-sync-client/changelog/fix-remove-use-of-ts-loader#2 @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Update build configuration to better match supported target environments. diff --git a/projects/js-packages/svelte-data-sync-client/package.json b/projects/js-packages/svelte-data-sync-client/package.json index a70c193cb72c2..0f97e1a256f8b 100644 --- a/projects/js-packages/svelte-data-sync-client/package.json +++ b/projects/js-packages/svelte-data-sync-client/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "@automattic/jetpack-svelte-data-sync-client", - "version": "0.3.5", + "version": "0.3.6-alpha", "description": "A Svelte.js client for the wp-js-data-sync package", "homepage": "https://github.com/Automattic/jetpack/tree/HEAD/projects/js-packages/svelte-data-sync-client/#readme", "type": "module", @@ -29,7 +29,6 @@ "eslint": "8.51.0", "jest": "29.7.0", "svelte": "3.58.0", - "ts-loader": "9.4.2", "tslib": "2.5.0", "typescript": "5.0.4", "webpack": "5.76.0", diff --git a/projects/js-packages/svelte-data-sync-client/src/DataSync.ts b/projects/js-packages/svelte-data-sync-client/src/DataSync.ts index 909e882e07a6c..6078418312657 100644 --- a/projects/js-packages/svelte-data-sync-client/src/DataSync.ts +++ b/projects/js-packages/svelte-data-sync-client/src/DataSync.ts @@ -1,7 +1,7 @@ import { z } from 'zod'; -import { ApiError } from './ApiError'; -import type { ParsedValue } from './types'; -import type { JSONSchema } from './utils'; +import { ApiError } from './ApiError.js'; +import type { ParsedValue } from './types.js'; +import type { JSONSchema } from './utils.js'; type RequestParams = string | JSONSchema; type RequestMethods = 'GET' | 'POST' | 'DELETE'; diff --git a/projects/js-packages/svelte-data-sync-client/src/SyncedStore.ts b/projects/js-packages/svelte-data-sync-client/src/SyncedStore.ts index f792808a466ef..f7d295c56ac44 100644 --- a/projects/js-packages/svelte-data-sync-client/src/SyncedStore.ts +++ b/projects/js-packages/svelte-data-sync-client/src/SyncedStore.ts @@ -1,13 +1,13 @@ import { Writable, writable } from 'svelte/store'; -import { ApiError } from './ApiError'; +import { ApiError } from './ApiError.js'; import { Pending, SyncedStoreInterface, SyncedWritable, SyncedStoreCallback, SyncedStoreError, -} from './types'; -import { sleep } from './utils'; +} from './types.js'; +import { sleep } from './utils.js'; /* * A custom Svelte Store that's used to indicate if a value is being synced. diff --git a/projects/js-packages/svelte-data-sync-client/src/index.ts b/projects/js-packages/svelte-data-sync-client/src/index.ts index 93a9bb8e5b83d..4c5b1695d4bef 100644 --- a/projects/js-packages/svelte-data-sync-client/src/index.ts +++ b/projects/js-packages/svelte-data-sync-client/src/index.ts @@ -1,9 +1,9 @@ -export { SyncedStore } from './SyncedStore'; -export { initializeClient } from './initializeClient'; +export { SyncedStore } from './SyncedStore.js'; +export { initializeClient } from './initializeClient.js'; export type { SyncedStoreCallback, SyncedStoreInterface, ParsedValue as ValidatedValue, SyncedWritable, SyncedStoreError, -} from './types'; +} from './types.js'; diff --git a/projects/js-packages/svelte-data-sync-client/src/initializeClient.ts b/projects/js-packages/svelte-data-sync-client/src/initializeClient.ts index e0029f5d3b479..34ed0f360c2f2 100644 --- a/projects/js-packages/svelte-data-sync-client/src/initializeClient.ts +++ b/projects/js-packages/svelte-data-sync-client/src/initializeClient.ts @@ -1,7 +1,7 @@ import { derived } from 'svelte/store'; import { z } from 'zod'; -import { DataSync } from './DataSync'; -import { SyncedStore } from './SyncedStore'; +import { DataSync } from './DataSync.js'; +import { SyncedStore } from './SyncedStore.js'; /** * Initialize the client-side data sync. diff --git a/projects/js-packages/svelte-data-sync-client/tsconfig.json b/projects/js-packages/svelte-data-sync-client/tsconfig.json index a88a0ca2461c4..f8a9be283f0de 100644 --- a/projects/js-packages/svelte-data-sync-client/tsconfig.json +++ b/projects/js-packages/svelte-data-sync-client/tsconfig.json @@ -7,8 +7,8 @@ "forceConsistentCasingInFileNames": true, "outDir": "./build/", "noEmit": false, - "declaration": true, - "module": "es6", - "target": "es5" + "module": "nodenext", + "moduleResolution": "nodenext", + "declaration": true } } diff --git a/projects/js-packages/svelte-data-sync-client/webpack.config.cjs b/projects/js-packages/svelte-data-sync-client/webpack.config.cjs index 5bdf6e462cb41..eb34b48319ca4 100644 --- a/projects/js-packages/svelte-data-sync-client/webpack.config.cjs +++ b/projects/js-packages/svelte-data-sync-client/webpack.config.cjs @@ -10,11 +10,10 @@ module.exports = { module: { strictExportPresence: true, rules: [ - { - test: /\.ts?$/, - use: 'ts-loader', - exclude: /node_modules/, - }, + // Transpile JavaScript and TypeScript + jetpackWebpackConfig.TranspileRule( { + exclude: /node_modules\//, + } ), ], }, optimization: { @@ -32,5 +31,10 @@ module.exports = { type: 'umd', }, }, - plugins: [ ...jetpackWebpackConfig.StandardPlugins() ], + plugins: [ + ...jetpackWebpackConfig.StandardPlugins( { + // Generate `.d.ts` files per tsconfig settings. + ForkTSCheckerPlugin: {}, + } ), + ], }; diff --git a/projects/js-packages/videopress-core/changelog/fix-remove-use-of-ts-loader b/projects/js-packages/videopress-core/changelog/fix-remove-use-of-ts-loader new file mode 100644 index 0000000000000..0ef77f6133eed --- /dev/null +++ b/projects/js-packages/videopress-core/changelog/fix-remove-use-of-ts-loader @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Update build configuration to better match supported target environments. diff --git a/projects/js-packages/videopress-core/package.json b/projects/js-packages/videopress-core/package.json index 07f2bba68550e..65d81388f0519 100644 --- a/projects/js-packages/videopress-core/package.json +++ b/projects/js-packages/videopress-core/package.json @@ -27,7 +27,6 @@ "@babel/core": "7.23.5", "@babel/preset-react": "7.23.3", "@types/jest": "29.5.12", - "ts-loader": "9.4.2", "tslib": "2.5.0", "typescript": "5.0.4", "webpack": "5.76.0", diff --git a/projects/js-packages/videopress-core/tsconfig.json b/projects/js-packages/videopress-core/tsconfig.json index a88a0ca2461c4..f8a9be283f0de 100644 --- a/projects/js-packages/videopress-core/tsconfig.json +++ b/projects/js-packages/videopress-core/tsconfig.json @@ -7,8 +7,8 @@ "forceConsistentCasingInFileNames": true, "outDir": "./build/", "noEmit": false, - "declaration": true, - "module": "es6", - "target": "es5" + "module": "nodenext", + "moduleResolution": "nodenext", + "declaration": true } } diff --git a/projects/js-packages/videopress-core/webpack.config.cjs b/projects/js-packages/videopress-core/webpack.config.cjs index b203b381d1aba..4164b260b4281 100644 --- a/projects/js-packages/videopress-core/webpack.config.cjs +++ b/projects/js-packages/videopress-core/webpack.config.cjs @@ -10,11 +10,10 @@ module.exports = { module: { strictExportPresence: true, rules: [ - { - test: /\.ts?$/, - use: 'ts-loader', - exclude: /node_modules/, - }, + // Transpile JavaScript and TypeScript + jetpackWebpackConfig.TranspileRule( { + exclude: /node_modules\//, + } ), ], }, optimization: { @@ -32,5 +31,10 @@ module.exports = { type: 'umd', }, }, - plugins: [ ...jetpackWebpackConfig.StandardPlugins() ], + plugins: [ + ...jetpackWebpackConfig.StandardPlugins( { + // Generate `.d.ts` files per tsconfig settings. + ForkTSCheckerPlugin: {}, + } ), + ], };