From feb857dd418826da25577d5bb5fcd8aba3e1e20c Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Fri, 14 Feb 2025 12:32:39 +0800 Subject: [PATCH 01/12] feat(type-checking): Add type-checking pre-commit hook for frontend Added the script as a pre-commit hook to ensure type-checking before commits. --- .pre-commit-config.yaml | 12 +- scripts/check-type.js | 176 ++++++++++++++++++++++++++++ superset-frontend/package-lock.json | 30 +++++ superset-frontend/package.json | 1 + 4 files changed, 218 insertions(+), 1 deletion(-) create mode 100755 scripts/check-type.js diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a40c26f218a1b..7f80fac9d02cd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -68,6 +68,16 @@ repos: language: system pass_filenames: true files: \.(js|jsx|ts|tsx)$ + - repo: local + hooks: + - id: type-checking-frontend + name: Type-Checking (Frontend) + entry: ./scripts/check-type.js + args: [package=superset-frontend, excludeDeclarationDir=cypress-base] + language: node + files: ^superset-frontend/ + exclude: ^superset-frontend/cypress-base/ + types_or: [ts, tsx, javascript, jsx] # blacklist unsafe functions like make_url (see #19526) - repo: https://github.com/skorokithakis/blacklist-pre-commit-hook rev: e2f070289d8eddcaec0b580d3bde29437e7c8221 @@ -83,5 +93,5 @@ repos: rev: v0.8.0 hooks: - id: ruff - args: [ --fix ] + args: [--fix] - id: ruff-format diff --git a/scripts/check-type.js b/scripts/check-type.js new file mode 100755 index 0000000000000..5d65cbf0769a5 --- /dev/null +++ b/scripts/check-type.js @@ -0,0 +1,176 @@ +#!/usr/bin/env node + +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// @ts-check +const { exit } = require("node:process"); +const { join, dirname } = require("node:path"); +const { readdirSync } = require("node:fs"); +const { chdir, cwd } = require("node:process"); +const { createRequire } = require("node:module"); + +const SUPERSET_ROOT = dirname(__dirname); +const PACKAGE_ARG_REGEX = /^package=/; +const EXCLUDE_DECLARATION_DIR_REGEX = /^excludeDeclarationDir=/; +const DECLARATION_FILE_REGEX = /\.d\.ts$/; + +void (async () => { + const args = process.argv.slice(2); + const { + matchedArgs: [packageArg, excludeDeclarationDirArg], + remainingArgs, + } = extractArgs(args, [PACKAGE_ARG_REGEX, EXCLUDE_DECLARATION_DIR_REGEX]); + + if (!packageArg) { + console.error("package is not specified"); + exit(1); + } + + const packageRootDir = getPackage(packageArg); + const packagePathRegex = new RegExp(`^${packageRootDir}\/`); + const updatedArgs = removePackageSegment(remainingArgs, packagePathRegex); + + const excludedDeclarationDirs = getExcludedDeclarationDirs( + excludeDeclarationDirArg + ); + let declarationFiles = getFilesRecursivelySync( + packageRootDir, + DECLARATION_FILE_REGEX, + excludedDeclarationDirs + ); + declarationFiles = removePackageSegment(declarationFiles, packagePathRegex); + + try { + chdir(join(SUPERSET_ROOT, packageRootDir)); + const packageRequire = createRequire(join(cwd(), "node_modules")); + // Please ensure that tscw-config is installed in the package being type-checked. + const tscw = packageRequire("tscw-config"); + const tsConfig = join(cwd(), "tsconfig.json"); + + const child = + await tscw`--noEmit --allowJs --project ${tsConfig} --composite false ${updatedArgs.join( + " " + )} ${declarationFiles.join(" ")}`; + + if (child.stdout) { + console.log(child.stdout); + } else { + console.log(child.stderr); + } + + exit(child.exitCode); + } catch (e) { + console.error(e); + exit(1); + } +})(); + +/** + * @param {string} dir + * @param {RegExp} regex + * @param {string[]} excludedDirs + * + * @returns {string[]} + */ +function getFilesRecursivelySync(dir, regex, excludedDirs) { + const files = readdirSync(dir, { withFileTypes: true }); + /** @type {string[]} */ + let result = []; + + for (const file of files) { + const fullPath = join(dir, file.name); + const shouldExclude = excludedDirs.includes(file.name); + + if (file.isDirectory() && !shouldExclude) { + result = result.concat( + getFilesRecursivelySync(fullPath, regex, excludedDirs) + ); + } else if (regex.test(file.name)) { + result.push(fullPath); + } + } + return result; +} + +/** + * + * @param {string} packageArg + * @returns {string} + */ +function getPackage(packageArg) { + return packageArg.split("=")[1].replace(/\/$/, ""); +} + +/** + * + * @param {string | undefined} excludeDeclarationDirArg + * @returns {string[]} + */ +function getExcludedDeclarationDirs(excludeDeclarationDirArg) { + const excludedDirs = ["node_modules"]; + + return !excludeDeclarationDirArg + ? excludedDirs + : excludeDeclarationDirArg + .split("=")[1] + .split(",") + .map((dir) => dir.replace(/\/$/, "").trim()) + .concat(excludedDirs); +} + +/** + * + * @param {string[]} args + * @param {RegExp[]} regexes + * @returns {{ matchedArgs: (string | undefined)[], remainingArgs: string[] }} + */ + +function extractArgs(args, regexes) { + /** + * @type {(string | undefined)[]} + */ + const matchedArgs = []; + const remainingArgs = [...args]; + + regexes.forEach((regex) => { + const index = remainingArgs.findIndex((arg) => regex.test(arg)); + if (index !== -1) { + const [arg] = remainingArgs.splice(index, 1); + matchedArgs.push(arg); + } else { + matchedArgs.push(undefined); + } + }); + + return { matchedArgs, remainingArgs }; +} + +/** + * Remove the package segment from path. + * + * For example: `superset-frontend/foo/bar.ts` -> `foo/bar.ts` + * + * @param {string[]} args + * @param {RegExp} packagePathRegex + * @returns {string[]} + */ +function removePackageSegment(args, packagePathRegex) { + return args.map((arg) => arg.replace(packagePathRegex, "")); +} diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 1e06523143977..6c83270e734e1 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -291,6 +291,7 @@ "thread-loader": "^4.0.4", "ts-jest": "^29.2.5", "ts-loader": "^9.5.1", + "tscw-config": "^1.1.1", "typescript": "5.1.6", "vm-browserify": "^1.1.2", "webpack": "^5.97.1", @@ -45970,6 +45971,35 @@ "node": ">=4" } }, + "node_modules/tscw-config": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", + "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "dev": true, + "license": "MIT", + "dependencies": { + "strip-json-comments": "^5.0.1" + }, + "bin": { + "tscw": "dist/cli.js" + }, + "peerDependencies": { + "typescript": ">=2.0.0" + } + }, + "node_modules/tscw-config/node_modules/strip-json-comments": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-5.0.1.tgz", + "integrity": "sha512-0fk9zBqO67Nq5M/m45qHCJxylV/DhBlIOVExqgOMiCCrzrhU6tCibRXNqE3jwJLftzE9SNuZtYbpzcO+i9FiKw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=14.16" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/tslib": { "version": "2.8.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 7bea4299f8f0e..8ef7c20903074 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -358,6 +358,7 @@ "thread-loader": "^4.0.4", "ts-jest": "^29.2.5", "ts-loader": "^9.5.1", + "tscw-config": "^1.1.1", "typescript": "5.1.6", "vm-browserify": "^1.1.2", "webpack": "^5.97.1", From 83c005daea10fea06e3144d6216cd81c00321795 Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Fri, 14 Feb 2025 12:50:15 +0800 Subject: [PATCH 02/12] feat(type-checking): Add type-checking pre-commit hook for websocket --- .pre-commit-config.yaml | 9 ++++++ superset-websocket/package-lock.json | 47 ++++++++++++++++++++++++++++ superset-websocket/package.json | 1 + superset-websocket/tsconfig.json | 2 +- 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7f80fac9d02cd..806d4529455af 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -78,6 +78,15 @@ repos: files: ^superset-frontend/ exclude: ^superset-frontend/cypress-base/ types_or: [ts, tsx, javascript, jsx] + - repo: local + hooks: + - id: type-checking-websocket + name: Type-Checking (Websocket) + entry: ./scripts/check-type.js + args: [package=superset-websocket] + language: node + files: ^superset-websocket/ + types_or: [ts, tsx, javascript, jsx] # blacklist unsafe functions like make_url (see #19526) - repo: https://github.com/skorokithakis/blacklist-pre-commit-hook rev: e2f070289d8eddcaec0b580d3bde29437e7c8221 diff --git a/superset-websocket/package-lock.json b/superset-websocket/package-lock.json index 00ed81f90b541..fc67fd9f217ca 100644 --- a/superset-websocket/package-lock.json +++ b/superset-websocket/package-lock.json @@ -39,6 +39,7 @@ "prettier": "^3.4.2", "ts-jest": "^29.2.5", "ts-node": "^10.9.2", + "tscw-config": "^1.1.1", "typescript": "^5.6.2", "typescript-eslint": "^8.19.0" }, @@ -6117,6 +6118,35 @@ "node": ">=0.4.0" } }, + "node_modules/tscw-config": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", + "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "dev": true, + "license": "MIT", + "dependencies": { + "strip-json-comments": "^5.0.1" + }, + "bin": { + "tscw": "dist/cli.js" + }, + "peerDependencies": { + "typescript": ">=2.0.0" + } + }, + "node_modules/tscw-config/node_modules/strip-json-comments": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-5.0.1.tgz", + "integrity": "sha512-0fk9zBqO67Nq5M/m45qHCJxylV/DhBlIOVExqgOMiCCrzrhU6tCibRXNqE3jwJLftzE9SNuZtYbpzcO+i9FiKw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=14.16" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/type-check": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/type-check/-/type-check-0.4.0.tgz", @@ -11038,6 +11068,23 @@ } } }, + "tscw-config": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", + "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "dev": true, + "requires": { + "strip-json-comments": "^5.0.1" + }, + "dependencies": { + "strip-json-comments": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-5.0.1.tgz", + "integrity": "sha512-0fk9zBqO67Nq5M/m45qHCJxylV/DhBlIOVExqgOMiCCrzrhU6tCibRXNqE3jwJLftzE9SNuZtYbpzcO+i9FiKw==", + "dev": true + } + } + }, "type-check": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/type-check/-/type-check-0.4.0.tgz", diff --git a/superset-websocket/package.json b/superset-websocket/package.json index 5bffd39487712..3881f7f3138b0 100644 --- a/superset-websocket/package.json +++ b/superset-websocket/package.json @@ -47,6 +47,7 @@ "prettier": "^3.4.2", "ts-jest": "^29.2.5", "ts-node": "^10.9.2", + "tscw-config": "^1.1.1", "typescript": "^5.6.2", "typescript-eslint": "^8.19.0" }, diff --git a/superset-websocket/tsconfig.json b/superset-websocket/tsconfig.json index 095e822d45eb4..346f8a0ee7d1b 100644 --- a/superset-websocket/tsconfig.json +++ b/superset-websocket/tsconfig.json @@ -8,5 +8,5 @@ "skipLibCheck": true, "forceConsistentCasingInFileNames": true }, - "include": ["src/**/*"], + "include": ["src/**/*"] } From 0936369591e7491760ba07aeec0d64933ff0baa1 Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Fri, 14 Feb 2025 12:55:10 +0800 Subject: [PATCH 03/12] feat(type-checking): Add type-checking pre-commit hook for embedded-sdk --- .pre-commit-config.yaml | 9 +++++ superset-embedded-sdk/package-lock.json | 51 ++++++++++++++++++++++++- superset-embedded-sdk/package.json | 1 + 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 806d4529455af..71449925a3ff8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -87,6 +87,15 @@ repos: language: node files: ^superset-websocket/ types_or: [ts, tsx, javascript, jsx] + - repo: local + hooks: + - id: type-checking-embedded-sdk + name: Type-Checking (Embedded SDK) + entry: ./scripts/check-type.js + args: [package=superset-embedded-sdk] + language: node + files: ^superset-embedded-sdk/ + types_or: [ts, tsx, javascript, jsx] # blacklist unsafe functions like make_url (see #19526) - repo: https://github.com/skorokithakis/blacklist-pre-commit-hook rev: e2f070289d8eddcaec0b580d3bde29437e7c8221 diff --git a/superset-embedded-sdk/package-lock.json b/superset-embedded-sdk/package-lock.json index 9d2b809ac29ab..070362f74b1a6 100644 --- a/superset-embedded-sdk/package-lock.json +++ b/superset-embedded-sdk/package-lock.json @@ -1,12 +1,12 @@ { "name": "@superset-ui/embedded-sdk", - "version": "0.1.2", + "version": "0.1.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@superset-ui/embedded-sdk", - "version": "0.1.2", + "version": "0.1.3", "license": "Apache-2.0", "dependencies": { "@superset-ui/switchboard": "^0.20.3", @@ -22,6 +22,7 @@ "axios": "^1.7.7", "babel-loader": "^9.1.3", "jest": "^29.7.0", + "tscw-config": "^1.1.1", "typescript": "^5.6.2", "webpack": "^5.94.0", "webpack-cli": "^5.1.4" @@ -7800,6 +7801,35 @@ "node": ">=8.0" } }, + "node_modules/tscw-config": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", + "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "dev": true, + "license": "MIT", + "dependencies": { + "strip-json-comments": "^5.0.1" + }, + "bin": { + "tscw": "dist/cli.js" + }, + "peerDependencies": { + "typescript": ">=2.0.0" + } + }, + "node_modules/tscw-config/node_modules/strip-json-comments": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-5.0.1.tgz", + "integrity": "sha512-0fk9zBqO67Nq5M/m45qHCJxylV/DhBlIOVExqgOMiCCrzrhU6tCibRXNqE3jwJLftzE9SNuZtYbpzcO+i9FiKw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=14.16" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/type-detect": { "version": "4.0.8", "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-4.0.8.tgz", @@ -13826,6 +13856,23 @@ "is-number": "^7.0.0" } }, + "tscw-config": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", + "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "dev": true, + "requires": { + "strip-json-comments": "^5.0.1" + }, + "dependencies": { + "strip-json-comments": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-5.0.1.tgz", + "integrity": "sha512-0fk9zBqO67Nq5M/m45qHCJxylV/DhBlIOVExqgOMiCCrzrhU6tCibRXNqE3jwJLftzE9SNuZtYbpzcO+i9FiKw==", + "dev": true + } + } + }, "type-detect": { "version": "4.0.8", "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-4.0.8.tgz", diff --git a/superset-embedded-sdk/package.json b/superset-embedded-sdk/package.json index 64bbae9d4405d..8c7377d48e8f4 100644 --- a/superset-embedded-sdk/package.json +++ b/superset-embedded-sdk/package.json @@ -46,6 +46,7 @@ "axios": "^1.7.7", "babel-loader": "^9.1.3", "jest": "^29.7.0", + "tscw-config": "^1.1.1", "typescript": "^5.6.2", "webpack": "^5.94.0", "webpack-cli": "^5.1.4" From 245b1a1b1983adb6604c970ce02ce7b888594f8d Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Fri, 14 Feb 2025 13:05:25 +0800 Subject: [PATCH 04/12] feat(type-checking): Add type-checking pre-commit hook for frontend/cypress --- .pre-commit-config.yaml | 9 +++ .../cypress-base/package-lock.json | 71 ++++++++++++++++++- superset-frontend/cypress-base/package.json | 3 +- .../types/{ace-builds.ts => ace-builds.d.ts} | 0 4 files changed, 81 insertions(+), 2 deletions(-) rename superset-frontend/src/types/{ace-builds.ts => ace-builds.d.ts} (100%) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 71449925a3ff8..50bfad65a1d6c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -96,6 +96,15 @@ repos: language: node files: ^superset-embedded-sdk/ types_or: [ts, tsx, javascript, jsx] + - repo: local + hooks: + - id: type-checking-cypress + name: Type-Checking (Cypress) + entry: ./scripts/check-type.js + args: [package=superset-frontend/cypress-base] + language: node + files: ^superset-frontend/cypress-base/ + types_or: [ts, tsx, javascript, jsx] # blacklist unsafe functions like make_url (see #19526) - repo: https://github.com/skorokithakis/blacklist-pre-commit-hook rev: e2f070289d8eddcaec0b580d3bde29437e7c8221 diff --git a/superset-frontend/cypress-base/package-lock.json b/superset-frontend/cypress-base/package-lock.json index 60719f539e254..b4ee4ce9eafed 100644 --- a/superset-frontend/cypress-base/package-lock.json +++ b/superset-frontend/cypress-base/package-lock.json @@ -24,7 +24,8 @@ "devDependencies": { "@types/querystringify": "^2.0.0", "cypress": "^11.2.0", - "eslint-plugin-cypress": "^3.5.0" + "eslint-plugin-cypress": "^3.5.0", + "tscw-config": "^1.1.1" } }, "node_modules/@ampproject/remapping": { @@ -10295,6 +10296,35 @@ "url": "https://github.com/sponsors/wooorm" } }, + "node_modules/tscw-config": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", + "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "dev": true, + "license": "MIT", + "dependencies": { + "strip-json-comments": "^5.0.1" + }, + "bin": { + "tscw": "dist/cli.js" + }, + "peerDependencies": { + "typescript": ">=2.0.0" + } + }, + "node_modules/tscw-config/node_modules/strip-json-comments": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-5.0.1.tgz", + "integrity": "sha512-0fk9zBqO67Nq5M/m45qHCJxylV/DhBlIOVExqgOMiCCrzrhU6tCibRXNqE3jwJLftzE9SNuZtYbpzcO+i9FiKw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=14.16" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/tslib": { "version": "1.14.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.14.1.tgz", @@ -10353,6 +10383,21 @@ "is-typedarray": "^1.0.0" } }, + "node_modules/typescript": { + "version": "4.9.5", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", + "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", + "devOptional": true, + "license": "Apache-2.0", + "peer": true, + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=4.2.0" + } + }, "node_modules/undici-types": { "version": "5.26.5", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-5.26.5.tgz", @@ -18602,6 +18647,23 @@ "resolved": "https://registry.npmjs.org/trough/-/trough-2.1.0.tgz", "integrity": "sha512-AqTiAOLcj85xS7vQ8QkAV41hPDIJ71XJB4RCUrzo/1GM2CQwhkJGaf9Hgr7BOugMRpgGUrqRg/DrBDl4H40+8g==" }, + "tscw-config": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", + "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "dev": true, + "requires": { + "strip-json-comments": "^5.0.1" + }, + "dependencies": { + "strip-json-comments": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-5.0.1.tgz", + "integrity": "sha512-0fk9zBqO67Nq5M/m45qHCJxylV/DhBlIOVExqgOMiCCrzrhU6tCibRXNqE3jwJLftzE9SNuZtYbpzcO+i9FiKw==", + "dev": true + } + } + }, "tslib": { "version": "1.14.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.14.1.tgz", @@ -18648,6 +18710,13 @@ "is-typedarray": "^1.0.0" } }, + "typescript": { + "version": "4.9.5", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.5.tgz", + "integrity": "sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g==", + "devOptional": true, + "peer": true + }, "undici-types": { "version": "5.26.5", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-5.26.5.tgz", diff --git a/superset-frontend/cypress-base/package.json b/superset-frontend/cypress-base/package.json index 9842d103ba251..1fb40877afd9b 100644 --- a/superset-frontend/cypress-base/package.json +++ b/superset-frontend/cypress-base/package.json @@ -31,6 +31,7 @@ "devDependencies": { "@types/querystringify": "^2.0.0", "cypress": "^11.2.0", - "eslint-plugin-cypress": "^3.5.0" + "eslint-plugin-cypress": "^3.5.0", + "tscw-config": "^1.1.1" } } diff --git a/superset-frontend/src/types/ace-builds.ts b/superset-frontend/src/types/ace-builds.d.ts similarity index 100% rename from superset-frontend/src/types/ace-builds.ts rename to superset-frontend/src/types/ace-builds.d.ts From 5040e52f896f43d4dc47aae6ffe096558ad0ea8d Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Sat, 15 Feb 2025 01:46:32 +0800 Subject: [PATCH 05/12] refactor(type-checking): correct log levels, improve performance, and add error context - Updated log levels for stdout and stderr to reflect appropriate severity. - Refactored fs operations to use asynchronous method. - Enhanced error handling by adding context information. --- .pre-commit-config.yaml | 14 +++----- scripts/check-type.js | 34 +++++++++++-------- superset-embedded-sdk/package-lock.json | 14 ++++---- superset-embedded-sdk/package.json | 2 +- .../cypress-base/package-lock.json | 14 ++++---- superset-frontend/cypress-base/package.json | 2 +- superset-frontend/package-lock.json | 8 ++--- superset-frontend/package.json | 2 +- superset-websocket/package-lock.json | 14 ++++---- superset-websocket/package.json | 2 +- 10 files changed, 53 insertions(+), 53 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 50bfad65a1d6c..fb0424a115a9a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -75,9 +75,8 @@ repos: entry: ./scripts/check-type.js args: [package=superset-frontend, excludeDeclarationDir=cypress-base] language: node - files: ^superset-frontend/ - exclude: ^superset-frontend/cypress-base/ - types_or: [ts, tsx, javascript, jsx] + files: ^superset-frontend\/.*\.(js|jsx|ts|tsx)$ + exclude: ^superset-frontend/cypress-base\/ - repo: local hooks: - id: type-checking-websocket @@ -85,8 +84,7 @@ repos: entry: ./scripts/check-type.js args: [package=superset-websocket] language: node - files: ^superset-websocket/ - types_or: [ts, tsx, javascript, jsx] + files: ^superset-websocket\/.*\.(js|jsx|ts|tsx)$ - repo: local hooks: - id: type-checking-embedded-sdk @@ -94,8 +92,7 @@ repos: entry: ./scripts/check-type.js args: [package=superset-embedded-sdk] language: node - files: ^superset-embedded-sdk/ - types_or: [ts, tsx, javascript, jsx] + files: ^superset-embedded-sdk\/.*\.(js|jsx|ts|tsx)$ - repo: local hooks: - id: type-checking-cypress @@ -103,8 +100,7 @@ repos: entry: ./scripts/check-type.js args: [package=superset-frontend/cypress-base] language: node - files: ^superset-frontend/cypress-base/ - types_or: [ts, tsx, javascript, jsx] + files: ^superset-frontend/cypress-base\/.*\.(js|jsx|ts|tsx)$ # blacklist unsafe functions like make_url (see #19526) - repo: https://github.com/skorokithakis/blacklist-pre-commit-hook rev: e2f070289d8eddcaec0b580d3bde29437e7c8221 diff --git a/scripts/check-type.js b/scripts/check-type.js index 5d65cbf0769a5..8a02ca818ce2a 100755 --- a/scripts/check-type.js +++ b/scripts/check-type.js @@ -22,7 +22,7 @@ // @ts-check const { exit } = require("node:process"); const { join, dirname } = require("node:path"); -const { readdirSync } = require("node:fs"); +const { readdir } = require("node:fs/promises"); const { chdir, cwd } = require("node:process"); const { createRequire } = require("node:module"); @@ -46,38 +46,41 @@ void (async () => { const packageRootDir = getPackage(packageArg); const packagePathRegex = new RegExp(`^${packageRootDir}\/`); const updatedArgs = removePackageSegment(remainingArgs, packagePathRegex); + const argsStr = updatedArgs.join(" "); const excludedDeclarationDirs = getExcludedDeclarationDirs( excludeDeclarationDirArg ); - let declarationFiles = getFilesRecursivelySync( + let declarationFiles = await getFilesRecursively( packageRootDir, DECLARATION_FILE_REGEX, excludedDeclarationDirs ); declarationFiles = removePackageSegment(declarationFiles, packagePathRegex); + const declarationFilesStr = declarationFiles.join(" "); + + const packageRootDirAbsolute = join(SUPERSET_ROOT, packageRootDir); + const tsConfig = join(packageRootDirAbsolute, "tsconfig.json"); + const command = `--noEmit --allowJs --composite false --project ${tsConfig} ${argsStr} ${declarationFilesStr}`; try { - chdir(join(SUPERSET_ROOT, packageRootDir)); + chdir(packageRootDirAbsolute); const packageRequire = createRequire(join(cwd(), "node_modules")); // Please ensure that tscw-config is installed in the package being type-checked. const tscw = packageRequire("tscw-config"); - const tsConfig = join(cwd(), "tsconfig.json"); - - const child = - await tscw`--noEmit --allowJs --project ${tsConfig} --composite false ${updatedArgs.join( - " " - )} ${declarationFiles.join(" ")}`; + const child = await tscw`${command}`; if (child.stdout) { console.log(child.stdout); } else { - console.log(child.stderr); + console.error(child.stderr); } exit(child.exitCode); } catch (e) { - console.error(e); + console.error("Failed to execute type checking:", e); + console.error("Package:", packageRootDir); + console.error("Command:", `tscw ${command}`); exit(1); } })(); @@ -87,10 +90,11 @@ void (async () => { * @param {RegExp} regex * @param {string[]} excludedDirs * - * @returns {string[]} + * @returns {Promise} */ -function getFilesRecursivelySync(dir, regex, excludedDirs) { - const files = readdirSync(dir, { withFileTypes: true }); + +async function getFilesRecursively(dir, regex, excludedDirs) { + const files = await readdir(dir, { withFileTypes: true }); /** @type {string[]} */ let result = []; @@ -100,7 +104,7 @@ function getFilesRecursivelySync(dir, regex, excludedDirs) { if (file.isDirectory() && !shouldExclude) { result = result.concat( - getFilesRecursivelySync(fullPath, regex, excludedDirs) + await getFilesRecursively(fullPath, regex, excludedDirs) ); } else if (regex.test(file.name)) { result.push(fullPath); diff --git a/superset-embedded-sdk/package-lock.json b/superset-embedded-sdk/package-lock.json index 070362f74b1a6..a8c3f84b97907 100644 --- a/superset-embedded-sdk/package-lock.json +++ b/superset-embedded-sdk/package-lock.json @@ -22,7 +22,7 @@ "axios": "^1.7.7", "babel-loader": "^9.1.3", "jest": "^29.7.0", - "tscw-config": "^1.1.1", + "tscw-config": "^1.1.2", "typescript": "^5.6.2", "webpack": "^5.94.0", "webpack-cli": "^5.1.4" @@ -7802,9 +7802,9 @@ } }, "node_modules/tscw-config": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", - "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.2.tgz", + "integrity": "sha512-mrrMxCqC6kjqjuhGc7mTOB3P7JuBebZ0ZnFQTi4e+K0K+2kT1OvTXzFygWCPBor/F8WJ1IWVRrnBLKctFhFwOQ==", "dev": true, "license": "MIT", "dependencies": { @@ -13857,9 +13857,9 @@ } }, "tscw-config": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", - "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.2.tgz", + "integrity": "sha512-mrrMxCqC6kjqjuhGc7mTOB3P7JuBebZ0ZnFQTi4e+K0K+2kT1OvTXzFygWCPBor/F8WJ1IWVRrnBLKctFhFwOQ==", "dev": true, "requires": { "strip-json-comments": "^5.0.1" diff --git a/superset-embedded-sdk/package.json b/superset-embedded-sdk/package.json index 8c7377d48e8f4..c18fab803a87b 100644 --- a/superset-embedded-sdk/package.json +++ b/superset-embedded-sdk/package.json @@ -46,7 +46,7 @@ "axios": "^1.7.7", "babel-loader": "^9.1.3", "jest": "^29.7.0", - "tscw-config": "^1.1.1", + "tscw-config": "^1.1.2", "typescript": "^5.6.2", "webpack": "^5.94.0", "webpack-cli": "^5.1.4" diff --git a/superset-frontend/cypress-base/package-lock.json b/superset-frontend/cypress-base/package-lock.json index b4ee4ce9eafed..c0cfd92ada4c4 100644 --- a/superset-frontend/cypress-base/package-lock.json +++ b/superset-frontend/cypress-base/package-lock.json @@ -25,7 +25,7 @@ "@types/querystringify": "^2.0.0", "cypress": "^11.2.0", "eslint-plugin-cypress": "^3.5.0", - "tscw-config": "^1.1.1" + "tscw-config": "^1.1.2" } }, "node_modules/@ampproject/remapping": { @@ -10297,9 +10297,9 @@ } }, "node_modules/tscw-config": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", - "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.2.tgz", + "integrity": "sha512-mrrMxCqC6kjqjuhGc7mTOB3P7JuBebZ0ZnFQTi4e+K0K+2kT1OvTXzFygWCPBor/F8WJ1IWVRrnBLKctFhFwOQ==", "dev": true, "license": "MIT", "dependencies": { @@ -18648,9 +18648,9 @@ "integrity": "sha512-AqTiAOLcj85xS7vQ8QkAV41hPDIJ71XJB4RCUrzo/1GM2CQwhkJGaf9Hgr7BOugMRpgGUrqRg/DrBDl4H40+8g==" }, "tscw-config": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", - "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.2.tgz", + "integrity": "sha512-mrrMxCqC6kjqjuhGc7mTOB3P7JuBebZ0ZnFQTi4e+K0K+2kT1OvTXzFygWCPBor/F8WJ1IWVRrnBLKctFhFwOQ==", "dev": true, "requires": { "strip-json-comments": "^5.0.1" diff --git a/superset-frontend/cypress-base/package.json b/superset-frontend/cypress-base/package.json index 1fb40877afd9b..c5defbfdfb414 100644 --- a/superset-frontend/cypress-base/package.json +++ b/superset-frontend/cypress-base/package.json @@ -32,6 +32,6 @@ "@types/querystringify": "^2.0.0", "cypress": "^11.2.0", "eslint-plugin-cypress": "^3.5.0", - "tscw-config": "^1.1.1" + "tscw-config": "^1.1.2" } } diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 6c83270e734e1..5c0f6e7c22cec 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -291,7 +291,7 @@ "thread-loader": "^4.0.4", "ts-jest": "^29.2.5", "ts-loader": "^9.5.1", - "tscw-config": "^1.1.1", + "tscw-config": "^1.1.2", "typescript": "5.1.6", "vm-browserify": "^1.1.2", "webpack": "^5.97.1", @@ -45972,9 +45972,9 @@ } }, "node_modules/tscw-config": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", - "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.2.tgz", + "integrity": "sha512-mrrMxCqC6kjqjuhGc7mTOB3P7JuBebZ0ZnFQTi4e+K0K+2kT1OvTXzFygWCPBor/F8WJ1IWVRrnBLKctFhFwOQ==", "dev": true, "license": "MIT", "dependencies": { diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 8ef7c20903074..c733ed799d56a 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -358,7 +358,7 @@ "thread-loader": "^4.0.4", "ts-jest": "^29.2.5", "ts-loader": "^9.5.1", - "tscw-config": "^1.1.1", + "tscw-config": "^1.1.2", "typescript": "5.1.6", "vm-browserify": "^1.1.2", "webpack": "^5.97.1", diff --git a/superset-websocket/package-lock.json b/superset-websocket/package-lock.json index fc67fd9f217ca..fc1bb31e252c3 100644 --- a/superset-websocket/package-lock.json +++ b/superset-websocket/package-lock.json @@ -39,7 +39,7 @@ "prettier": "^3.4.2", "ts-jest": "^29.2.5", "ts-node": "^10.9.2", - "tscw-config": "^1.1.1", + "tscw-config": "^1.1.2", "typescript": "^5.6.2", "typescript-eslint": "^8.19.0" }, @@ -6119,9 +6119,9 @@ } }, "node_modules/tscw-config": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", - "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.2.tgz", + "integrity": "sha512-mrrMxCqC6kjqjuhGc7mTOB3P7JuBebZ0ZnFQTi4e+K0K+2kT1OvTXzFygWCPBor/F8WJ1IWVRrnBLKctFhFwOQ==", "dev": true, "license": "MIT", "dependencies": { @@ -11069,9 +11069,9 @@ } }, "tscw-config": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.1.tgz", - "integrity": "sha512-5Q+1iopqkQpFWXNXr6S95hCqBmJLRy9J6Tojhnz1is4+mgQ+17iEXqWJ+EGHLc9YduCW+CMY15k6ZZ1sSLCJjA==", + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/tscw-config/-/tscw-config-1.1.2.tgz", + "integrity": "sha512-mrrMxCqC6kjqjuhGc7mTOB3P7JuBebZ0ZnFQTi4e+K0K+2kT1OvTXzFygWCPBor/F8WJ1IWVRrnBLKctFhFwOQ==", "dev": true, "requires": { "strip-json-comments": "^5.0.1" diff --git a/superset-websocket/package.json b/superset-websocket/package.json index 3881f7f3138b0..57e3020d27cef 100644 --- a/superset-websocket/package.json +++ b/superset-websocket/package.json @@ -47,7 +47,7 @@ "prettier": "^3.4.2", "ts-jest": "^29.2.5", "ts-node": "^10.9.2", - "tscw-config": "^1.1.1", + "tscw-config": "^1.1.2", "typescript": "^5.6.2", "typescript-eslint": "^8.19.0" }, From 503378e5bf4e8b0ae54044e5699588756041a7d1 Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Sat, 15 Feb 2025 13:45:10 +0800 Subject: [PATCH 06/12] fix(type-checking): Fix regular expression injection vulnerability --- scripts/check-type.js | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/scripts/check-type.js b/scripts/check-type.js index 8a02ca818ce2a..41fcb9fa739b4 100755 --- a/scripts/check-type.js +++ b/scripts/check-type.js @@ -21,7 +21,7 @@ // @ts-check const { exit } = require("node:process"); -const { join, dirname } = require("node:path"); +const { join, dirname, normalize, sep } = require("node:path"); const { readdir } = require("node:fs/promises"); const { chdir, cwd } = require("node:process"); const { createRequire } = require("node:module"); @@ -44,8 +44,7 @@ void (async () => { } const packageRootDir = getPackage(packageArg); - const packagePathRegex = new RegExp(`^${packageRootDir}\/`); - const updatedArgs = removePackageSegment(remainingArgs, packagePathRegex); + const updatedArgs = removePackageSegment(remainingArgs, packageRootDir); const argsStr = updatedArgs.join(" "); const excludedDeclarationDirs = getExcludedDeclarationDirs( @@ -56,7 +55,7 @@ void (async () => { DECLARATION_FILE_REGEX, excludedDeclarationDirs ); - declarationFiles = removePackageSegment(declarationFiles, packagePathRegex); + declarationFiles = removePackageSegment(declarationFiles, packageRootDir); const declarationFilesStr = declarationFiles.join(" "); const packageRootDirAbsolute = join(SUPERSET_ROOT, packageRootDir); @@ -172,9 +171,17 @@ function extractArgs(args, regexes) { * For example: `superset-frontend/foo/bar.ts` -> `foo/bar.ts` * * @param {string[]} args - * @param {RegExp} packagePathRegex + * @param {string} package * @returns {string[]} */ -function removePackageSegment(args, packagePathRegex) { - return args.map((arg) => arg.replace(packagePathRegex, "")); +function removePackageSegment(args, package) { + const packageSegment = package.concat(sep); + return args.map((arg) => { + const normalizedPath = normalize(arg); + + if (normalizedPath.startsWith(packageSegment)) { + return normalizedPath.slice(packageSegment.length); + } + return arg; + }); } From f79234295198ddcb71fc6dab059710a520d591c5 Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Sat, 15 Feb 2025 13:48:29 +0800 Subject: [PATCH 07/12] chore(type-checking): remove unnecessary pre-commit hooks - Removed type-checking-websocket hook - Removed type-checking-embedded-sdk hook - Removed type-checking-cypress hook --- .pre-commit-config.yaml | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index fb0424a115a9a..eb88910febba1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -68,8 +68,6 @@ repos: language: system pass_filenames: true files: \.(js|jsx|ts|tsx)$ - - repo: local - hooks: - id: type-checking-frontend name: Type-Checking (Frontend) entry: ./scripts/check-type.js @@ -77,30 +75,6 @@ repos: language: node files: ^superset-frontend\/.*\.(js|jsx|ts|tsx)$ exclude: ^superset-frontend/cypress-base\/ - - repo: local - hooks: - - id: type-checking-websocket - name: Type-Checking (Websocket) - entry: ./scripts/check-type.js - args: [package=superset-websocket] - language: node - files: ^superset-websocket\/.*\.(js|jsx|ts|tsx)$ - - repo: local - hooks: - - id: type-checking-embedded-sdk - name: Type-Checking (Embedded SDK) - entry: ./scripts/check-type.js - args: [package=superset-embedded-sdk] - language: node - files: ^superset-embedded-sdk\/.*\.(js|jsx|ts|tsx)$ - - repo: local - hooks: - - id: type-checking-cypress - name: Type-Checking (Cypress) - entry: ./scripts/check-type.js - args: [package=superset-frontend/cypress-base] - language: node - files: ^superset-frontend/cypress-base\/.*\.(js|jsx|ts|tsx)$ # blacklist unsafe functions like make_url (see #19526) - repo: https://github.com/skorokithakis/blacklist-pre-commit-hook rev: e2f070289d8eddcaec0b580d3bde29437e7c8221 From ae43454b6520680afeaae2953d06077d6894b454 Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Sun, 16 Feb 2025 00:24:04 +0800 Subject: [PATCH 08/12] chore(ci): disable type-checking-frontend pre-commit hook execution in CI pipeline Disable it as it requires npm ci, plus type-check is handled by other job. --- .github/workflows/pre-commit.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index f3044353f7637..b231aeb81ef18 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -41,8 +41,8 @@ jobs: - name: pre-commit run: | set +e # Don't exit immediately on failure - # Skip eslint as it requires `npm ci` and is executed in another job - export SKIP=eslint + # Skip eslint and type-checking-frontend as they requires `npm ci`. Both eslint and type-check are handled in other jobs. + export SKIP=eslint,type-checking-frontend pre-commit run --all-files if [ $? -ne 0 ] || ! git diff --quiet --exit-code; then echo "❌ Pre-commit check failed." From fdb4290ecf7b5424ae5e5e96ed9ccd6ffe011c32 Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Tue, 18 Feb 2025 00:31:37 +0800 Subject: [PATCH 09/12] fix(type-checking): improve error handling and process outputs - Add check for module installation and provide clear error message if missing - Ensure both stdout and stderr are logged if they contain content - Add check for existence of tsconfig.json and provide clear error message if missing --- scripts/check-type.js | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/scripts/check-type.js b/scripts/check-type.js index 41fcb9fa739b4..e0cc9d8f1884f 100755 --- a/scripts/check-type.js +++ b/scripts/check-type.js @@ -23,6 +23,7 @@ const { exit } = require("node:process"); const { join, dirname, normalize, sep } = require("node:path"); const { readdir } = require("node:fs/promises"); +const { existsSync } = require("node:fs"); const { chdir, cwd } = require("node:process"); const { createRequire } = require("node:module"); @@ -59,19 +60,20 @@ void (async () => { const declarationFilesStr = declarationFiles.join(" "); const packageRootDirAbsolute = join(SUPERSET_ROOT, packageRootDir); - const tsConfig = join(packageRootDirAbsolute, "tsconfig.json"); + const tsConfig = getTsConfig(packageRootDirAbsolute); const command = `--noEmit --allowJs --composite false --project ${tsConfig} ${argsStr} ${declarationFilesStr}`; try { chdir(packageRootDirAbsolute); - const packageRequire = createRequire(join(cwd(), "node_modules")); // Please ensure that tscw-config is installed in the package being type-checked. const tscw = packageRequire("tscw-config"); const child = await tscw`${command}`; if (child.stdout) { console.log(child.stdout); - } else { + } + + if (child.stderr) { console.error(child.stderr); } @@ -185,3 +187,34 @@ function removePackageSegment(args, package) { return arg; }); } + +/** + * + * @param {string} dir + */ +function getTsConfig(dir) { + const defaultTsConfig = "tsconfig.json"; + const tsConfig = join(dir, defaultTsConfig); + + if (!existsSync(tsConfig)) { + console.error(`Error: ${defaultTsConfig} not found in ${dir}`); + exit(1); + } + return tsConfig; +} + +/** + * + * @param {string} module + */ +function packageRequire(module) { + try { + const localRequire = createRequire(join(cwd(), "node_modules")); + return localRequire(module); + } catch (e) { + console.error( + `Error: ${module} is not installed in ${cwd()}. Please install it first.` + ); + exit(1); + } +} From 0bed7b2cf5eebec62ec2d3cf1c9c73ba2bd8b2ba Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Tue, 18 Feb 2025 16:28:13 +0800 Subject: [PATCH 10/12] fix(type-checking): Enhance functions for error handling and performance - getFilesRecursively: use `promise.all`, handle error. - getPackage: handle error. --- scripts/check-type.js | 62 +++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 20 deletions(-) diff --git a/scripts/check-type.js b/scripts/check-type.js index e0cc9d8f1884f..46ae11af0e0f3 100755 --- a/scripts/check-type.js +++ b/scripts/check-type.js @@ -22,7 +22,7 @@ // @ts-check const { exit } = require("node:process"); const { join, dirname, normalize, sep } = require("node:path"); -const { readdir } = require("node:fs/promises"); +const { readdir, stat } = require("node:fs/promises"); const { existsSync } = require("node:fs"); const { chdir, cwd } = require("node:process"); const { createRequire } = require("node:module"); @@ -44,7 +44,7 @@ void (async () => { exit(1); } - const packageRootDir = getPackage(packageArg); + const packageRootDir = await getPackage(packageArg); const updatedArgs = removePackageSegment(remainingArgs, packageRootDir); const argsStr = updatedArgs.join(" "); @@ -95,32 +95,54 @@ void (async () => { */ async function getFilesRecursively(dir, regex, excludedDirs) { - const files = await readdir(dir, { withFileTypes: true }); - /** @type {string[]} */ - let result = []; - - for (const file of files) { - const fullPath = join(dir, file.name); - const shouldExclude = excludedDirs.includes(file.name); - - if (file.isDirectory() && !shouldExclude) { - result = result.concat( - await getFilesRecursively(fullPath, regex, excludedDirs) - ); - } else if (regex.test(file.name)) { - result.push(fullPath); + try { + const files = await readdir(dir, { withFileTypes: true }); + const recursivePromises = []; + const result = []; + + for (const file of files) { + const fullPath = join(dir, file.name); + const shouldExclude = excludedDirs.includes(file.name); + + if (file.isDirectory() && !shouldExclude) { + recursivePromises.push( + getFilesRecursively(fullPath, regex, excludedDirs) + ); + } else if (regex.test(file.name)) { + result.push(fullPath); + } } + + const recursiveResults = await Promise.all(recursivePromises); + return result.concat(...recursiveResults); + } catch (e) { + console.error(`Error reading directory: ${dir}`); + console.error(e); + exit(1); } - return result; } /** * * @param {string} packageArg - * @returns {string} + * @returns {Promise} */ -function getPackage(packageArg) { - return packageArg.split("=")[1].replace(/\/$/, ""); +async function getPackage(packageArg) { + const packageDir = packageArg.split("=")[1].replace(/\/$/, ""); + try { + const stats = await stat(packageDir); + if (!stats.isDirectory()) { + console.error( + `Please specify a valid package, ${packageDir} is not a directory.` + ); + exit(1); + } + } catch (e) { + console.error(`Error reading package: ${packageDir}`); + console.error(e); + exit(1); + } + return packageDir; } /** From d97b699b5411cfcdbb07adfa24ae547af998305e Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Tue, 18 Feb 2025 17:29:55 +0800 Subject: [PATCH 11/12] refactor(type-checking): update exclusion logic for more fine-grained controll --- scripts/check-type.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/check-type.js b/scripts/check-type.js index 46ae11af0e0f3..a066ceae2d285 100755 --- a/scripts/check-type.js +++ b/scripts/check-type.js @@ -102,7 +102,9 @@ async function getFilesRecursively(dir, regex, excludedDirs) { for (const file of files) { const fullPath = join(dir, file.name); - const shouldExclude = excludedDirs.includes(file.name); + const shouldExclude = excludedDirs.some((excludedDir) => + normalize(fullPath).includes(normalize(excludedDir)) + ); if (file.isDirectory() && !shouldExclude) { recursivePromises.push( From 1bbd40f5a86021bbab1a84b443f9943ebe3a6e1e Mon Sep 17 00:00:00 2001 From: alveifbklsiu259 Date: Wed, 19 Feb 2025 18:47:26 +0800 Subject: [PATCH 12/12] fix(type-checking): fix exclusion logic false positives --- scripts/check-type.js | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/scripts/check-type.js b/scripts/check-type.js index a066ceae2d285..dcd93993c4391 100755 --- a/scripts/check-type.js +++ b/scripts/check-type.js @@ -86,6 +86,25 @@ void (async () => { } })(); +/** + * + * @param {string} fullPath + * @param {string[]} excludedDirs + */ +function shouldExcludeDir(fullPath, excludedDirs) { + return excludedDirs.some((excludedDir) => { + const normalizedExcludedDir = normalize(excludedDir); + const normalizedPath = normalize(fullPath); + return ( + normalizedExcludedDir === normalizedPath || + normalizedPath + .split(sep) + .filter((segment) => segment) + .includes(normalizedExcludedDir) + ); + }); +} + /** * @param {string} dir * @param {RegExp} regex @@ -102,11 +121,8 @@ async function getFilesRecursively(dir, regex, excludedDirs) { for (const file of files) { const fullPath = join(dir, file.name); - const shouldExclude = excludedDirs.some((excludedDir) => - normalize(fullPath).includes(normalize(excludedDir)) - ); - if (file.isDirectory() && !shouldExclude) { + if (file.isDirectory() && !shouldExcludeDir(fullPath, excludedDirs)) { recursivePromises.push( getFilesRecursively(fullPath, regex, excludedDirs) );