Skip to content

Commit

Permalink
build: additional fixes for tsetse rule compliance
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
clydin committed Jun 25, 2024
1 parent 7c5b365 commit 17e1683
Show file tree
Hide file tree
Showing 18 changed files with 46 additions and 24 deletions.
15 changes: 15 additions & 0 deletions .yarn/patches/@bazel-concatjs-npm-5.8.1-1bf81df846.patch
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ async function findCustomJestConfig(dir: string): Promise<string | undefined> {
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
};
}

Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion packages/ngtools/webpack/src/ivy/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ export function angularWebpackLoader(this: LoaderContext<unknown>, 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<typeof callback>[2],
string | undefined
>;
resultMap.sources = resultMap.sources.map((source: string) =>
path.join(path.dirname(this.resourcePath), source),
);
Expand Down
4 changes: 2 additions & 2 deletions tests/legacy-cli/e2e/tests/basic/command-scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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\./,
);
Expand All @@ -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\./,
);
Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/tests/basic/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' : '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/tests/i18n/extract-ivy-disk-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' };
});
Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/utils/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/utils/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
}

Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
}
Expand Down
9 changes: 8 additions & 1 deletion tsconfig-test.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
}
]
}
}
3 changes: 0 additions & 3 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@
"@schematics/angular": ["./packages/schematics/angular"]
}
},
"bazelOptions": {
"suppressTsconfigOverrideWarnings": true
},
"exclude": [
"packages/angular_devkit/build_angular/src/babel-bazel.d.ts",
"dist/**/*",
Expand Down
4 changes: 2 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand Down

0 comments on commit 17e1683

Please sign in to comment.