From bf899ef0f485f288d38d949dbb15d07db41a20a4 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 20 Mar 2025 12:11:09 +0000 Subject: [PATCH] build: prepare testing infrastructure for code splitting of `core` package (#60487) When we switch to relative imports, shared `.d.ts` chunks can be generated. We need to also pull these into our mock virtual FS testing environments. Notably this does not cause a test slow-down because we are talking about very few extra `.d.ts` chunk files. In our experiments before, with no dts bundling, we saw test time increase from e.g. 20seconds to 100seconds. The 20s are still the same locally! In addition, since code for definitions can now reside in shared `.d.ts` chunks, the language service tests need to be adjusted in cases where they assert for code definition locations in `@angular/core`. A new helper prepares for more code to be moved into arbitrary `.d.ts` files; we should simply assert the definition comes out of `node_modules/@angular/core`. PR Close #60487 --- .../src/ngtsc/typecheck/testing/BUILD.bazel | 1 + .../src/ngtsc/typecheck/testing/index.ts | 81 +++++-------------- .../test/type_definitions_spec.ts | 5 +- packages/language-service/testing/src/util.ts | 36 ++++++++- 4 files changed, 59 insertions(+), 64 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/testing/BUILD.bazel index 4f42571d9db..f1eb155145b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/BUILD.bazel @@ -23,6 +23,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/typecheck", "//packages/compiler-cli/src/ngtsc/typecheck/api", "//packages/compiler-cli/src/ngtsc/util", + "@npm//tinyglobby", "@npm//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index bec007fc322..ce3c17a8082 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -24,6 +24,7 @@ import { import {readFileSync} from 'fs'; import path from 'path'; import ts from 'typescript'; +import {globSync} from 'tinyglobby'; import { absoluteFrom, @@ -153,19 +154,19 @@ export function typescriptLibDts(): TestFile { }; } +let _angularCoreDts: TestFile[] | null = null; export function angularCoreDtsFiles(): TestFile[] { - const directory = resolveFromRunfiles('angular/packages/core/npm_package'); + if (_angularCoreDts !== null) { + return _angularCoreDts; + } - return [ - { - name: absoluteFrom('/node_modules/@angular/core/index.d.ts'), - contents: readFileSync(path.join(directory, 'index.d.ts'), 'utf8'), - }, - { - name: absoluteFrom('/node_modules/@angular/core/primitives/signals/index.d.ts'), - contents: readFileSync(path.join(directory, 'primitives/signals/index.d.ts'), 'utf8'), - }, - ]; + const directory = resolveFromRunfiles('angular/packages/core/npm_package'); + const dtsFiles = globSync('**/*.d.ts', {cwd: directory}); + + return (_angularCoreDts = dtsFiles.map((fileName) => ({ + name: absoluteFrom(`/node_modules/@angular/core/${fileName}`), + contents: readFileSync(path.join(directory, fileName), 'utf8'), + }))); } export function angularAnimationsDts(): TestFile { @@ -248,12 +249,7 @@ export function ngForDts(): TestFile { export function ngForTypeCheckTarget(): TypeCheckingTarget { const dts = ngForDts(); - return { - ...dts, - fileName: dts.name, - source: dts.contents, - templates: {}, - }; + return {...dts, fileName: dts.name, source: dts.contents, templates: {}}; } export const ALL_ENABLED_CONFIG: Readonly = { @@ -427,9 +423,7 @@ export function tcb( checkTwoWayBoundEvents: true, ...config, }; - options = options || { - emitSpans: false, - }; + options = options || {emitSpans: false}; const fileName = absoluteFrom('/type-check-file.ts'); @@ -520,18 +514,12 @@ export function setup( } } - files.push({ - name: target.fileName, - contents, - }); + files.push({name: target.fileName, contents}); if (!target.fileName.endsWith('.d.ts')) { const shimName = TypeCheckShimGenerator.shimFor(target.fileName); shims.set(target.fileName, shimName); - files.push({ - name: shimName, - contents: 'export const MODULE = true;', - }); + files.push({name: shimName, contents: 'export const MODULE = true;'}); } } @@ -540,12 +528,7 @@ export function setup( const {program, host, options} = makeProgram( files, - { - strictNullChecks: true, - skipLibCheck: true, - noImplicitAny: true, - ...opts, - }, + {strictNullChecks: true, skipLibCheck: true, noImplicitAny: true, ...opts}, /* host */ undefined, /* checkForErrors */ false, ); @@ -587,10 +570,7 @@ export function setup( if (shims.has(target.fileName)) { const shimFileName = shims.get(target.fileName)!; const shimSf = getSourceFileOrError(program, shimFileName); - sfExtensionData(shimSf).fileShim = { - extension: 'ngtypecheck', - generatedFrom: target.fileName, - }; + sfExtensionData(shimSf).fileShim = {extension: 'ngtypecheck', generatedFrom: target.fileName}; } for (const className of Object.keys(target.templates)) { @@ -671,10 +651,7 @@ export function setup( if (!scopeMap.has(clazz)) { // This class wasn't part of the target set of components with templates, but is // probably a declaration used in one of them. Return an empty scope. - const emptyScope: ScopeData = { - dependencies: [], - isPoisoned: false, - }; + const emptyScope: ScopeData = {dependencies: [], isPoisoned: false}; return { kind: ComponentScopeKind.NgModule, ngModule, @@ -725,11 +702,7 @@ export function setup( typeCheckScopeRegistry, NOOP_PERF_RECORDER, ); - return { - templateTypeChecker, - program, - programStrategy, - }; + return {templateTypeChecker, program, programStrategy}; } /** @@ -748,14 +721,7 @@ export function diagnose( const sfPath = absoluteFrom('/main.ts'); const {program, templateTypeChecker} = setup( [ - { - fileName: sfPath, - templates: { - 'TestComponent': template, - }, - source, - declarations, - }, + {fileName: sfPath, templates: {'TestComponent': template}, source, declarations}, ...additionalSources.map((testFile) => ({ fileName: testFile.name, source: testFile.contents, @@ -910,10 +876,7 @@ function getDirectiveMetaFromDeclaration( * Synthesize `ScopeData` metadata from an array of `TestDeclaration`s. */ function makeScope(program: ts.Program, sf: ts.SourceFile, decls: TestDeclaration[]): ScopeData { - const scope: ScopeData = { - dependencies: [], - isPoisoned: false, - }; + const scope: ScopeData = {dependencies: [], isPoisoned: false}; for (const decl of decls) { let declSf = sf; diff --git a/packages/language-service/test/type_definitions_spec.ts b/packages/language-service/test/type_definitions_spec.ts index ae4396b59d3..ac169076efc 100644 --- a/packages/language-service/test/type_definitions_spec.ts +++ b/packages/language-service/test/type_definitions_spec.ts @@ -10,6 +10,7 @@ import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/te import { assertFileNames, + assertFilePaths, assertTextSpans, humanizeDocumentSpanLike, LanguageServiceTestEnv, @@ -119,7 +120,7 @@ describe('type definitions', () => { expect(definitions!.length).toEqual(1); assertTextSpans(definitions, ['OutputEmitterRef']); - assertFileNames(definitions, ['index.d.ts']); + assertFilePaths(definitions, [/node_modules\/@angular\/core\/.*\.d\.ts/]); }); }); @@ -158,7 +159,7 @@ describe('type definitions', () => { expect(definitions!.length).toEqual(1); assertTextSpans(definitions, ['OutputRef']); - assertFileNames(definitions, ['index.d.ts']); + assertFilePaths(definitions, [/node_modules\/@angular\/core\/.*\.d\.ts/]); }); }); diff --git a/packages/language-service/testing/src/util.ts b/packages/language-service/testing/src/util.ts index 5fe31df2d9a..e32ae3d3977 100644 --- a/packages/language-service/testing/src/util.ts +++ b/packages/language-service/testing/src/util.ts @@ -22,6 +22,38 @@ export function assertFileNames(refs: Array<{fileName: string}>, expectedFileNam expect(new Set(actualFileNames)).toEqual(new Set(expectedFileNames)); } +/** + * Expect that a list of objects with a `fileName` property matches a set + * of file paths. + * + * This assertion is independent of the order of either list. + */ +export function assertFilePaths(refs: Array<{fileName: string}>, expectedPaths: RegExp[]) { + const actualPaths = Array.from(new Set(refs.map((r) => r.fileName))); + + if (actualPaths.length !== expectedPaths.length) { + expect(actualPaths.length) + .withContext('Expected expected paths to be the same size.') + .toBe(expectedPaths.length); + return; + } + + for (const pattern of expectedPaths) { + const matching = actualPaths.findIndex((p) => pattern.test(p)); + if (matching !== -1) { + actualPaths.splice(matching, 1); + } else { + expect(true) + .withContext( + `Expected ${pattern} to match a file path. ` + + `Remaining unmatched paths: ${actualPaths.join(', ')}`, + ) + .toBe(false); + return; + } + } +} + export function assertTextSpans(items: Array<{textSpan: string}>, expectedTextSpans: string[]) { const actualSpans = items.map((item) => item.textSpan); expect(new Set(actualSpans)).toEqual(new Set(expectedTextSpans)); @@ -97,9 +129,7 @@ export function humanizeDocumentSpanLike( : undefined, }; } -type Stringy = { - [P in keyof T]: string; -}; +type Stringy = {[P in keyof T]: string}; export function getText(contents: string, textSpan: ts.TextSpan) { return contents.slice(textSpan.start, textSpan.start + textSpan.length);