Skip to content

Commit a02e270

Browse files
devversionAndrewKushnir
authored andcommitted
build: properly compile tests in core with Angular compiler (angular#60268)
Previously we never could use relative imports to import e.g. `Component` in e.g. the `core/tests/bundling` folder. This was necessary because otherwise the Angular compiler wouldn't process those files as it wouldn't recognize the Angular decorator as the one from `@angular/core`. Notably this still isn't a large issue because relative imports still work for most core tests, that are JIT compiled! For bundling tests though, or some smaller targets, our new upcoming guidelines for using relative imports inside the full package; fall apart. This commit unblocks this effort and allows us to use relative imports in all tests of `packages/core`. This is achieved by leveraging the existing `isCore` functionality of the compiler, and fixing a few instances that were missing before. PR Close angular#60268
1 parent 9080fdf commit a02e270

File tree

11 files changed

+85
-42
lines changed

11 files changed

+85
-42
lines changed

packages/bazel/src/ng_module/ng_module.bzl

+6-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
"""Run Angular's AOT template compiler
66
"""
77

8-
load("//packages/bazel/src/ng_module:partial_compilation.bzl", "NgPartialCompilationInfo")
98
load(
109
"//packages/bazel/src:external.bzl",
1110
"COMMON_ATTRIBUTES",
@@ -23,6 +22,7 @@ load(
2322
"ts_providers_dict_to_struct",
2423
"tsc_wrapped_tsconfig",
2524
)
25+
load("//packages/bazel/src/ng_module:partial_compilation.bzl", "NgPartialCompilationInfo")
2626

2727
# enable_perf_logging controls whether Ivy's performance tracing system will be enabled for any
2828
# compilation which includes this provider.
@@ -180,6 +180,7 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs):
180180
# for aliased exports. We disable relative paths and always use manifest paths in google3.
181181
"_useHostForImportGeneration": (not _is_bazel()),
182182
"_useManifestPathsAsModuleName": (not _is_bazel()),
183+
"_isAngularCoreCompilation": ctx.attr.is_angular_core_compilation,
183184
}
184185

185186
if is_perf_requested(ctx):
@@ -474,6 +475,10 @@ NG_MODULE_ATTRIBUTES = {
474475
executable = True,
475476
cfg = "exec",
476477
),
478+
"is_angular_core_compilation": attr.bool(
479+
default = False,
480+
doc = "Whether this is a compilation of Angular core.",
481+
),
477482
"_partial_compilation_flag": attr.label(
478483
default = "//packages/bazel/src:partial_compilation",
479484
providers = [NgPartialCompilationInfo],

packages/compiler-cli/private/migrations.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* package requires for migration schematics.
1212
*/
1313

14-
export {forwardRefResolver} from '../src/ngtsc/annotations';
14+
export {createForwardRefResolver} from '../src/ngtsc/annotations';
1515
export {AbsoluteFsPath} from '../src/ngtsc/file_system';
1616
export {Reference} from '../src/ngtsc/imports';
1717
export {

packages/compiler-cli/src/ngtsc/annotations/common/src/evaluation.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,16 @@ export function resolveEnumValue(
2222
metadata: Map<string, ts.Expression>,
2323
field: string,
2424
enumSymbolName: string,
25+
isCore: boolean,
2526
): number | null {
2627
let resolved: number | null = null;
2728
if (metadata.has(field)) {
2829
const expr = metadata.get(field)!;
2930
const value = evaluator.evaluate(expr) as any;
30-
if (value instanceof EnumValue && isAngularCoreReference(value.enumRef, enumSymbolName)) {
31+
if (
32+
value instanceof EnumValue &&
33+
isAngularCoreReference(value.enumRef, enumSymbolName, isCore)
34+
) {
3135
resolved = value.resolved as number;
3236
} else {
3337
throw createValueHasWrongTypeError(

packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts

+21-18
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,14 @@ export function isAngularCore(decorator: Decorator): decorator is Decorator & {i
107107
return decorator.import !== null && decorator.import.from === CORE_MODULE;
108108
}
109109

110-
export function isAngularCoreReference(reference: Reference, symbolName: string): boolean {
111-
return reference.ownedByModuleGuess === CORE_MODULE && reference.debugName === symbolName;
110+
export function isAngularCoreReference(
111+
reference: Reference,
112+
symbolName: string,
113+
isCore: boolean,
114+
): boolean {
115+
return (
116+
(reference.ownedByModuleGuess === CORE_MODULE || isCore) && reference.debugName === symbolName
117+
);
112118
}
113119

114120
export function findAngularDecorator(
@@ -225,22 +231,19 @@ export function tryUnwrapForwardRef(
225231
* @param args the arguments to the invocation of the forwardRef expression
226232
* @returns an unwrapped argument if `ref` pointed to forwardRef, or null otherwise
227233
*/
228-
export const forwardRefResolver: ForeignFunctionResolver = (
229-
fn,
230-
callExpr,
231-
resolve,
232-
unresolvable,
233-
) => {
234-
if (!isAngularCoreReference(fn, 'forwardRef') || callExpr.arguments.length !== 1) {
235-
return unresolvable;
236-
}
237-
const expanded = expandForwardRef(callExpr.arguments[0]);
238-
if (expanded !== null) {
239-
return resolve(expanded);
240-
} else {
241-
return unresolvable;
242-
}
243-
};
234+
export function createForwardRefResolver(isCore: boolean): ForeignFunctionResolver {
235+
return (fn, callExpr, resolve, unresolvable) => {
236+
if (!isAngularCoreReference(fn, 'forwardRef', isCore) || callExpr.arguments.length !== 1) {
237+
return unresolvable;
238+
}
239+
const expanded = expandForwardRef(callExpr.arguments[0]);
240+
if (expanded !== null) {
241+
return resolve(expanded);
242+
} else {
243+
return unresolvable;
244+
}
245+
};
246+
}
244247

245248
/**
246249
* Combines an array of resolver functions into a one.

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,11 @@ import {
132132
compileInputTransformFields,
133133
compileNgFactoryDefField,
134134
compileResults,
135+
createForwardRefResolver,
135136
extractClassDebugInfo,
136137
extractClassMetadata,
137138
extractSchemas,
138139
findAngularDecorator,
139-
forwardRefResolver,
140140
getDirectiveDiagnostics,
141141
getProviderDiagnostics,
142142
InjectableClassRegistry,
@@ -492,7 +492,13 @@ export class ComponentDecoratorHandler
492492
} = directiveResult;
493493
const encapsulation: number =
494494
(this.compilationMode !== CompilationMode.LOCAL
495-
? resolveEnumValue(this.evaluator, component, 'encapsulation', 'ViewEncapsulation')
495+
? resolveEnumValue(
496+
this.evaluator,
497+
component,
498+
'encapsulation',
499+
'ViewEncapsulation',
500+
this.isCore,
501+
)
496502
: resolveEncapsulationEnumValueLocally(component.get('encapsulation'))) ??
497503
ViewEncapsulation.Emulated;
498504

@@ -503,6 +509,7 @@ export class ComponentDecoratorHandler
503509
component,
504510
'changeDetection',
505511
'ChangeDetectionStrategy',
512+
this.isCore,
506513
);
507514
} else if (component.has('changeDetection')) {
508515
changeDetection = new o.WrappedNodeExpr(component.get('changeDetection')!);
@@ -597,7 +604,7 @@ export class ComponentDecoratorHandler
597604
) {
598605
const importResolvers = combineResolvers([
599606
createModuleWithProvidersResolver(this.reflector, this.isCore),
600-
forwardRefResolver,
607+
createForwardRefResolver(this.isCore),
601608
]);
602609

603610
const importDiagnostics: ts.Diagnostic[] = [];

packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts

+9-2
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
import {
4848
DynamicValue,
4949
EnumValue,
50+
ForeignFunctionResolver,
5051
PartialEvaluator,
5152
ResolvedValue,
5253
traceDynamicValue,
@@ -65,9 +66,9 @@ import {
6566
import {CompilationMode} from '../../../transform';
6667
import {
6768
assertLocalCompilationUnresolvedConst,
69+
createForwardRefResolver,
6870
createSourceSpan,
6971
createValueHasWrongTypeError,
70-
forwardRefResolver,
7172
getAngularDecorators,
7273
getConstructorDependencies,
7374
isAngularDecorator,
@@ -373,7 +374,12 @@ export function extractDirectiveMetadata(
373374
const hostDirectives =
374375
rawHostDirectives === null
375376
? null
376-
: extractHostDirectives(rawHostDirectives, evaluator, compilationMode);
377+
: extractHostDirectives(
378+
rawHostDirectives,
379+
evaluator,
380+
compilationMode,
381+
createForwardRefResolver(isCore),
382+
);
377383

378384
if (compilationMode !== CompilationMode.LOCAL && hostDirectives !== null) {
379385
// In global compilation mode where we do type checking, the template type-checker will need to
@@ -1672,6 +1678,7 @@ function extractHostDirectives(
16721678
rawHostDirectives: ts.Expression,
16731679
evaluator: PartialEvaluator,
16741680
compilationMode: CompilationMode,
1681+
forwardRefResolver: ForeignFunctionResolver,
16751682
): HostDirectiveMeta[] {
16761683
const resolved = evaluator.evaluate(rawHostDirectives, forwardRefResolver);
16771684
if (!Array.isArray(resolved)) {

packages/compiler-cli/src/ngtsc/annotations/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
/// <reference types="node" />
1010

1111
export {
12-
forwardRefResolver,
12+
createForwardRefResolver,
1313
findAngularDecorator,
1414
getAngularDecorators,
1515
isAngularDecorator,

packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,11 @@ import {
9191
combineResolvers,
9292
compileDeclareFactory,
9393
compileNgFactoryDefField,
94+
createForwardRefResolver,
9495
createValueHasWrongTypeError,
9596
extractClassMetadata,
9697
extractSchemas,
9798
findAngularDecorator,
98-
forwardRefResolver,
9999
getProviderDiagnostics,
100100
getValidConstructorDependencies,
101101
InjectableClassRegistry,
@@ -348,6 +348,7 @@ export class NgModuleDecoratorHandler
348348
return {};
349349
}
350350

351+
const forwardRefResolver = createForwardRefResolver(this.isCore);
351352
const moduleResolvers = combineResolvers([
352353
createModuleWithProvidersResolver(this.reflector, this.isCore),
353354
forwardRefResolver,

packages/compiler-cli/src/ngtsc/core/api/src/options.ts

+9
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,15 @@ export interface InternalOptions {
126126
* Whether to check the event side of two-way bindings.
127127
*/
128128
_checkTwoWayBoundEvents?: boolean;
129+
130+
/**
131+
* Whether this is a compilation of Angular core itself.
132+
*
133+
* By default, we detect this automatically based on the existence of `r3_symbols.ts`
134+
* in the compilation, but there are other test targets within the `core` package that
135+
* import e.g. `Component` relatively and should be detected by the compiler.
136+
*/
137+
_isAngularCoreCompilation?: boolean;
129138
}
130139

131140
/**

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1257,7 +1257,8 @@ export class NgCompiler {
12571257
}
12581258

12591259
private makeCompilation(): LazyCompilationState {
1260-
const isCore = isAngularCorePackage(this.inputProgram);
1260+
const isCore =
1261+
this.options._isAngularCoreCompilation ?? isAngularCorePackage(this.inputProgram);
12611262

12621263
// Note: If this compilation builds `@angular/core`, we always build in full compilation
12631264
// mode. Code inside the core package is always compatible with itself, so it does not

tools/defaults.bzl

+19-13
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,26 @@
11
"""Re-export of some bazel rules with repository-wide defaults."""
22

3-
load("@rules_pkg//:pkg.bzl", "pkg_tar")
43
load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test", _npm_package_bin = "npm_package_bin", _pkg_npm = "pkg_npm")
5-
load("@npm//@bazel/jasmine:index.bzl", _jasmine_node_test = "jasmine_node_test")
6-
load("@npm//@bazel/concatjs:index.bzl", _ts_config = "ts_config", _ts_library = "ts_library")
7-
load("@npm//@bazel/rollup:index.bzl", _rollup_bundle = "rollup_bundle")
8-
load("@npm//@bazel/terser:index.bzl", "terser_minified")
9-
load("@npm//@bazel/protractor:index.bzl", _protractor_web_test_suite = "protractor_web_test_suite")
10-
load("@npm//typescript:index.bzl", "tsc")
11-
load("@npm//@angular/build-tooling/bazel/app-bundling:index.bzl", _app_bundle = "app_bundle")
12-
load("@npm//@angular/build-tooling/bazel/http-server:index.bzl", _http_server = "http_server")
13-
load("@npm//@angular/build-tooling/bazel/karma:index.bzl", _karma_web_test = "karma_web_test", _karma_web_test_suite = "karma_web_test_suite")
14-
load("@npm//@angular/build-tooling/bazel/api-golden:index.bzl", _api_golden_test = "api_golden_test", _api_golden_test_npm_package = "api_golden_test_npm_package")
154
load("@npm//@angular/build-tooling/bazel:extract_js_module_output.bzl", "extract_js_module_output")
165
load("@npm//@angular/build-tooling/bazel:extract_types.bzl", _extract_types = "extract_types")
6+
load("@npm//@angular/build-tooling/bazel/api-golden:index.bzl", _api_golden_test = "api_golden_test", _api_golden_test_npm_package = "api_golden_test_npm_package")
7+
load("@npm//@angular/build-tooling/bazel/app-bundling:index.bzl", _app_bundle = "app_bundle")
178
load("@npm//@angular/build-tooling/bazel/esbuild:index.bzl", _esbuild = "esbuild", _esbuild_config = "esbuild_config", _esbuild_esm_bundle = "esbuild_esm_bundle")
18-
load("@npm//@angular/build-tooling/bazel/spec-bundling:spec-entrypoint.bzl", "spec_entrypoint")
9+
load("@npm//@angular/build-tooling/bazel/http-server:index.bzl", _http_server = "http_server")
10+
load("@npm//@angular/build-tooling/bazel/karma:index.bzl", _karma_web_test = "karma_web_test", _karma_web_test_suite = "karma_web_test_suite")
1911
load("@npm//@angular/build-tooling/bazel/spec-bundling:index.bzl", "spec_bundle")
12+
load("@npm//@angular/build-tooling/bazel/spec-bundling:spec-entrypoint.bzl", "spec_entrypoint")
13+
load("@npm//@bazel/concatjs:index.bzl", _ts_config = "ts_config", _ts_library = "ts_library")
14+
load("@npm//@bazel/jasmine:index.bzl", _jasmine_node_test = "jasmine_node_test")
15+
load("@npm//@bazel/protractor:index.bzl", _protractor_web_test_suite = "protractor_web_test_suite")
16+
load("@npm//@bazel/rollup:index.bzl", _rollup_bundle = "rollup_bundle")
17+
load("@npm//@bazel/terser:index.bzl", "terser_minified")
2018
load("@npm//tsec:index.bzl", _tsec_test = "tsec_test")
19+
load("@npm//typescript:index.bzl", "tsc")
20+
load("@rules_pkg//:pkg.bzl", "pkg_tar")
21+
load("//adev/shared-docs/pipeline/api-gen:generate_api_docs.bzl", _generate_api_docs = "generate_api_docs")
2122
load("//packages/bazel:index.bzl", _ng_module = "ng_module", _ng_package = "ng_package")
2223
load("//tools/esm-interop:index.bzl", "enable_esm_node_module_loader", _nodejs_binary = "nodejs_binary", _nodejs_test = "nodejs_test")
23-
load("//adev/shared-docs/pipeline/api-gen:generate_api_docs.bzl", _generate_api_docs = "generate_api_docs")
2424

2525
_DEFAULT_TSCONFIG_TEST = "//packages:tsconfig-test"
2626
_INTERNAL_NG_MODULE_COMPILER = "//packages/bazel/src/ngc-wrapped"
@@ -170,6 +170,11 @@ def ng_module(name, tsconfig = None, entry_point = None, testonly = False, deps
170170

171171
if not entry_point:
172172
entry_point = "public_api.ts"
173+
174+
is_angular_core_compilation = False
175+
if native.package_name().startswith("packages/core"):
176+
is_angular_core_compilation = True
177+
173178
_ng_module(
174179
name = name,
175180
flat_module_out_file = name,
@@ -184,6 +189,7 @@ def ng_module(name, tsconfig = None, entry_point = None, testonly = False, deps
184189
# `package_name` can be set to allow for the Bazel NodeJS linker to run. This
185190
# allows for resolution of the given target within the `node_modules/`.
186191
package_name = package_name,
192+
is_angular_core_compilation = is_angular_core_compilation,
187193
perf_flag = "//packages/compiler-cli:ng_perf",
188194
**kwargs
189195
)

0 commit comments

Comments
 (0)