From 17e168379e9765cde240557ab38e3e1ea3ed9a6b Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 25 Jun 2024 09:40:27 -0400 Subject: [PATCH] build: additional fixes for tsetse rule compliance Due to bazel rules_nodejs caching, several additional `JSON.parse` usages were not caught in the first set of fixes. These have now been addressed. Also, the `must-use-promises` rule has been patched to match the behavior of the `@typescript-eslint/no-floating-promises` for consistency. The bazel option `suppressTsconfigOverrideWarnings` was also removed from the `tsconfig` as it is a no-op and was previously used for now removed feature. Test files are currently excluded from the `JSON.parse` rule to avoid large changes to test code. --- .../@bazel-concatjs-npm-5.8.1-1bf81df846.patch | 15 +++++++++++++++ .../build/src/utils/index-file/inline-fonts.ts | 2 +- .../architect/node/node-modules-architect-host.ts | 2 +- .../architect/src/jobs/simple-scheduler.ts | 2 +- .../build_angular/src/builders/jest/index.ts | 2 +- .../plugins/javascript-optimizer-worker.ts | 8 ++++---- packages/ngtools/webpack/src/ivy/loader.ts | 5 ++++- tests/legacy-cli/e2e/tests/basic/command-scope.ts | 4 ++-- tests/legacy-cli/e2e/tests/basic/e2e.ts | 2 +- .../e2e/tests/commands/add/version-specifier.ts | 2 +- .../project-cannot-be-determined-by-cwd.ts | 2 +- .../e2e/tests/i18n/extract-ivy-disk-cache.ts | 2 +- tests/legacy-cli/e2e/utils/project.ts | 2 +- tests/legacy-cli/e2e/utils/version.ts | 2 +- tests/legacy-cli/e2e_runner.ts | 2 +- tsconfig-test.json | 9 ++++++++- tsconfig.json | 3 --- yarn.lock | 4 ++-- 18 files changed, 46 insertions(+), 24 deletions(-) diff --git a/.yarn/patches/@bazel-concatjs-npm-5.8.1-1bf81df846.patch b/.yarn/patches/@bazel-concatjs-npm-5.8.1-1bf81df846.patch index 3a183b166bc1..8ea4fc259ae6 100644 --- a/.yarn/patches/@bazel-concatjs-npm-5.8.1-1bf81df846.patch +++ b/.yarn/patches/@bazel-concatjs-npm-5.8.1-1bf81df846.patch @@ -52,3 +52,18 @@ index b01c999f5e02b388f51a508b0b608cf69db9b664..847c23fe4829d0c847e9b4bd1ad698e1 } # "node_modules" still checked for backward compat for ng_module +diff --git a/internal/tsetse/rules/must_use_promises_rule.js b/internal/tsetse/rules/must_use_promises_rule.js +index e404d01cf80ab4da4b9cca89005b14a60b7d8c79..85488d9a339982af4495d2b5f4c30effb98a538b 100755 +--- a/internal/tsetse/rules/must_use_promises_rule.js ++++ b/internal/tsetse/rules/must_use_promises_rule.js +@@ -30,6 +30,10 @@ function checkCallExpression(checker, node) { + if (tsutils.isExpressionValueUsed(node) || !inAsyncFunction(node)) { + return; + } ++ // Sync behavior with @typescript-eslint/no-floating-promises ++ if (node.parent && ts.isVoidExpression(node.parent)) { ++ return; ++ } + if (tsutils.isThenableType(checker.typeChecker, node)) { + checker.addFailureAtNode(node, FAILURE_STRING); + } diff --git a/packages/angular/build/src/utils/index-file/inline-fonts.ts b/packages/angular/build/src/utils/index-file/inline-fonts.ts index 9322a343e4bb..a6f9e8452daf 100644 --- a/packages/angular/build/src/utils/index-file/inline-fonts.ts +++ b/packages/angular/build/src/utils/index-file/inline-fonts.ts @@ -97,7 +97,7 @@ export class InlineFontsProcessor { } }); - initCollectorStream().catch(() => { + void initCollectorStream().catch(() => { // We don't really care about any errors here because it just initializes // the rewriting stream, as we are waiting for `finish` below. }); diff --git a/packages/angular_devkit/architect/node/node-modules-architect-host.ts b/packages/angular_devkit/architect/node/node-modules-architect-host.ts index 21ac1199b2bd..682c3917af16 100644 --- a/packages/angular_devkit/architect/node/node-modules-architect-host.ts +++ b/packages/angular_devkit/architect/node/node-modules-architect-host.ts @@ -28,7 +28,7 @@ function clone(obj: unknown): unknown { try { return deserialize(serialize(obj)); } catch { - return JSON.parse(JSON.stringify(obj)); + return JSON.parse(JSON.stringify(obj)) as unknown; } } diff --git a/packages/angular_devkit/architect/src/jobs/simple-scheduler.ts b/packages/angular_devkit/architect/src/jobs/simple-scheduler.ts index c5a5d01ecb91..45cf32e169e8 100644 --- a/packages/angular_devkit/architect/src/jobs/simple-scheduler.ts +++ b/packages/angular_devkit/architect/src/jobs/simple-scheduler.ts @@ -147,7 +147,7 @@ export class SimpleScheduler< const description: JobDescription = { // Make a copy of it to be sure it's proper JSON. - ...JSON.parse(JSON.stringify(handler.jobDescription)), + ...(JSON.parse(JSON.stringify(handler.jobDescription)) as JobDescription), name: handler.jobDescription.name || name, argument: handler.jobDescription.argument || true, input: handler.jobDescription.input || true, diff --git a/packages/angular_devkit/build_angular/src/builders/jest/index.ts b/packages/angular_devkit/build_angular/src/builders/jest/index.ts index 505e4cab62f1..18008d450918 100644 --- a/packages/angular_devkit/build_angular/src/builders/jest/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/jest/index.ts @@ -204,7 +204,7 @@ async function findCustomJestConfig(dir: string): Promise { return undefined; // No package.json, therefore no Jest configuration in it. } - const json = JSON.parse(packageJson); + const json = JSON.parse(packageJson) as { jest?: unknown }; if ('jest' in json) { return packageJsonPath; } diff --git a/packages/angular_devkit/build_angular/src/tools/webpack/plugins/javascript-optimizer-worker.ts b/packages/angular_devkit/build_angular/src/tools/webpack/plugins/javascript-optimizer-worker.ts index 2a822d6d025e..ae22dbbd9d9a 100644 --- a/packages/angular_devkit/build_angular/src/tools/webpack/plugins/javascript-optimizer-worker.ts +++ b/packages/angular_devkit/build_angular/src/tools/webpack/plugins/javascript-optimizer-worker.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import remapping from '@ampproject/remapping'; +import remapping, { SourceMapInput } from '@ampproject/remapping'; import type { BuildFailure, TransformResult } from 'esbuild'; import { minify } from 'terser'; import { EsbuildExecutor } from './esbuild-executor'; @@ -79,7 +79,7 @@ interface OptimizeRequest { * The source map of the JavaScript asset, if available. * This map is merged with all intermediate source maps during optimization. */ - map: object; + map: SourceMapInput; }; } @@ -118,11 +118,11 @@ export default async function ({ asset, options }: OptimizeRequest) { const partialSourcemaps = []; if (esbuildResult.map) { - partialSourcemaps.unshift(JSON.parse(esbuildResult.map)); + partialSourcemaps.unshift(JSON.parse(esbuildResult.map) as SourceMapInput); } if (terserResult.map) { - partialSourcemaps.unshift(terserResult.map); + partialSourcemaps.unshift(terserResult.map as SourceMapInput); } if (asset.map) { diff --git a/packages/ngtools/webpack/src/ivy/loader.ts b/packages/ngtools/webpack/src/ivy/loader.ts index 1f5cf6124ee4..6c88a5babb5c 100644 --- a/packages/ngtools/webpack/src/ivy/loader.ts +++ b/packages/ngtools/webpack/src/ivy/loader.ts @@ -58,7 +58,10 @@ export function angularWebpackLoader(this: LoaderContext, content: stri let resultMap; if (result.map) { resultContent = resultContent.replace(/^\/\/# sourceMappingURL=[^\r\n]*/gm, ''); - resultMap = JSON.parse(result.map); + resultMap = JSON.parse(result.map) as Exclude< + Parameters[2], + string | undefined + >; resultMap.sources = resultMap.sources.map((source: string) => path.join(path.dirname(this.resourcePath), source), ); diff --git a/tests/legacy-cli/e2e/tests/basic/command-scope.ts b/tests/legacy-cli/e2e/tests/basic/command-scope.ts index 1a8c70dff561..f83ad21112b4 100644 --- a/tests/legacy-cli/e2e/tests/basic/command-scope.ts +++ b/tests/legacy-cli/e2e/tests/basic/command-scope.ts @@ -9,7 +9,7 @@ export default async function () { // The version command can be run in and outside of a workspace. await silentNg('version'); - assert.rejects( + await assert.rejects( silentNg('new', 'proj-name', '--dry-run'), /This command is not available when running the Angular CLI inside a workspace\./, ); @@ -18,7 +18,7 @@ export default async function () { process.chdir(homedir()); // ng generate can only be ran inside. - assert.rejects( + await assert.rejects( silentNg('generate', 'component', 'foo', '--dry-run'), /This command is not available when running the Angular CLI outside a workspace\./, ); diff --git a/tests/legacy-cli/e2e/tests/basic/e2e.ts b/tests/legacy-cli/e2e/tests/basic/e2e.ts index 0faf2f1973e9..655b679192d1 100644 --- a/tests/legacy-cli/e2e/tests/basic/e2e.ts +++ b/tests/legacy-cli/e2e/tests/basic/e2e.ts @@ -3,7 +3,7 @@ import { setTimeout } from 'node:timers/promises'; import { silentNg } from '../../utils/process'; export default async function () { - assert.rejects(silentNg('e2e', 'test-project', '--dev-server-target=')); + await assert.rejects(silentNg('e2e', 'test-project', '--dev-server-target=')); // These should work. await silentNg('e2e', 'test-project'); diff --git a/tests/legacy-cli/e2e/tests/commands/add/version-specifier.ts b/tests/legacy-cli/e2e/tests/commands/add/version-specifier.ts index 426f8d5b61e5..1729823c08e2 100644 --- a/tests/legacy-cli/e2e/tests/commands/add/version-specifier.ts +++ b/tests/legacy-cli/e2e/tests/commands/add/version-specifier.ts @@ -12,7 +12,7 @@ export default async function () { // `ng add` command itself and not the behavior of npm which may otherwise fail depending // on the npm version in use and the version specifier supplied in each test. if (getActivePackageManager() === 'npm') { - appendFile('.npmrc', '\nforce=true\n'); + await appendFile('.npmrc', '\nforce=true\n'); } const tag = (await isPrereleaseCli()) ? '@next' : ''; diff --git a/tests/legacy-cli/e2e/tests/commands/project-cannot-be-determined-by-cwd.ts b/tests/legacy-cli/e2e/tests/commands/project-cannot-be-determined-by-cwd.ts index 40ccce6640da..1cb6fbd6a5eb 100644 --- a/tests/legacy-cli/e2e/tests/commands/project-cannot-be-determined-by-cwd.ts +++ b/tests/legacy-cli/e2e/tests/commands/project-cannot-be-determined-by-cwd.ts @@ -25,7 +25,7 @@ export default async function () { } // Help should still work - execAndWaitForOutputToMatch('ng', ['build', '--help'], /--configuration/); + await execAndWaitForOutputToMatch('ng', ['build', '--help'], /--configuration/); // Yargs allows positional args to be passed as flags. Verify that in this case the project can be determined. await ng('build', '--project=third-app', '--configuration=development'); diff --git a/tests/legacy-cli/e2e/tests/i18n/extract-ivy-disk-cache.ts b/tests/legacy-cli/e2e/tests/i18n/extract-ivy-disk-cache.ts index 1c2aeea31675..da15bd2f5ff5 100644 --- a/tests/legacy-cli/e2e/tests/i18n/extract-ivy-disk-cache.ts +++ b/tests/legacy-cli/e2e/tests/i18n/extract-ivy-disk-cache.ts @@ -8,7 +8,7 @@ import { readNgVersion } from '../../utils/version'; export default async function () { // Enable disk cache - updateJsonFile('angular.json', (config) => { + await updateJsonFile('angular.json', (config) => { config.cli ??= {}; config.cli.cache = { environment: 'all' }; }); diff --git a/tests/legacy-cli/e2e/utils/project.ts b/tests/legacy-cli/e2e/utils/project.ts index 2438dec48d0e..80830940f6a2 100644 --- a/tests/legacy-cli/e2e/utils/project.ts +++ b/tests/legacy-cli/e2e/utils/project.ts @@ -12,7 +12,7 @@ import { execAndWaitForOutputToMatch, git, ng } from './process'; export function updateJsonFile(filePath: string, fn: (json: any) => any | void) { return readFile(filePath).then((tsConfigJson) => { // Remove single and multiline comments - const tsConfig = JSON.parse(tsConfigJson.replace(/\/\*\s(.|\n|\r)*\s\*\/|\/\/.*/g, '')); + const tsConfig = JSON.parse(tsConfigJson.replace(/\/\*\s(.|\n|\r)*\s\*\/|\/\/.*/g, '')) as any; const result = fn(tsConfig) || tsConfig; return writeFile(filePath, JSON.stringify(result, null, 2)); diff --git a/tests/legacy-cli/e2e/utils/version.ts b/tests/legacy-cli/e2e/utils/version.ts index 0ad0150d3483..e48e3a12a08d 100644 --- a/tests/legacy-cli/e2e/utils/version.ts +++ b/tests/legacy-cli/e2e/utils/version.ts @@ -4,7 +4,7 @@ import * as semver from 'semver'; export function readNgVersion(): string { const packageJson: any = JSON.parse( fs.readFileSync('./node_modules/@angular/core/package.json', 'utf8'), - ); + ) as { version: string }; return packageJson['version']; } diff --git a/tests/legacy-cli/e2e_runner.ts b/tests/legacy-cli/e2e_runner.ts index 0f10333119c1..ac9bad24d91d 100644 --- a/tests/legacy-cli/e2e_runner.ts +++ b/tests/legacy-cli/e2e_runner.ts @@ -371,7 +371,7 @@ async function findPackageTars(): Promise<{ [pkg: string]: PkgInfo }> { return pkgs.reduce( (all, pkg, i) => { const json = pkgJsons[i].toString('utf8'); - const { name, version } = JSON.parse(json); + const { name, version } = JSON.parse(json) as { name: string; version: string }; if (!name) { throw new Error(`Package ${pkg} - package.json name/version not found`); } diff --git a/tsconfig-test.json b/tsconfig-test.json index cae29f3a3cac..3881877cacbf 100644 --- a/tsconfig-test.json +++ b/tsconfig-test.json @@ -7,6 +7,13 @@ // Istanbul (not Constantinople) as well, and applying both source maps to get the original // source in devtools. "inlineSources": true, - "types": ["node", "jasmine"] + "types": ["node", "jasmine"], + "plugins": [ + { + "name": "@bazel/tsetse", + // TODO: Cleanup tests and remove this rule disable + "disabledRules": ["must-type-assert-json-parse"] + } + ] } } diff --git a/tsconfig.json b/tsconfig.json index f8324145a894..045ed0acdf3a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -33,9 +33,6 @@ "@schematics/angular": ["./packages/schematics/angular"] } }, - "bazelOptions": { - "suppressTsconfigOverrideWarnings": true - }, "exclude": [ "packages/angular_devkit/build_angular/src/babel-bazel.d.ts", "dist/**/*", diff --git a/yarn.lock b/yarn.lock index 84c1e2183e40..d7bb638322a6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2426,7 +2426,7 @@ __metadata: "@bazel/concatjs@patch:@bazel/concatjs@npm%3A5.8.1#~/.yarn/patches/@bazel-concatjs-npm-5.8.1-1bf81df846.patch": version: 5.8.1 - resolution: "@bazel/concatjs@patch:@bazel/concatjs@npm%3A5.8.1#~/.yarn/patches/@bazel-concatjs-npm-5.8.1-1bf81df846.patch::version=5.8.1&hash=6673ab" + resolution: "@bazel/concatjs@patch:@bazel/concatjs@npm%3A5.8.1#~/.yarn/patches/@bazel-concatjs-npm-5.8.1-1bf81df846.patch::version=5.8.1&hash=13d359" dependencies: protobufjs: "npm:6.8.8" source-map-support: "npm:0.5.9" @@ -2441,7 +2441,7 @@ __metadata: karma-sourcemap-loader: ">=0.3.0" bin: tsc_wrapped: internal/tsc_wrapped/tsc_wrapped.js - checksum: 10c0/644891b24514c83d9006f6a90abc0c4fc59816bdff56ed4fb4bd6c611ff4dc187c91dee112d98f1f694352b93691c97542aa04834fceabd44679540a5528a889 + checksum: 10c0/6b51579124bdc15650603d6a621fab7b11d7d4185d0eaa113ec23cad337436b89d690880a9285968e100e405678cc610b59ee335681e13035709dc444cc89733 languageName: node linkType: hard