Skip to content

Commit

Permalink
build: properly compile tests in core with Angular compiler (angula…
Browse files Browse the repository at this point in the history
…r#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
  • Loading branch information
devversion authored and AndrewKushnir committed Mar 7, 2025
1 parent 9080fdf commit a02e270
Show file tree
Hide file tree
Showing 11 changed files with 85 additions and 42 deletions.
7 changes: 6 additions & 1 deletion packages/bazel/src/ng_module/ng_module.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"""Run Angular's AOT template compiler
"""

load("//packages/bazel/src/ng_module:partial_compilation.bzl", "NgPartialCompilationInfo")
load(
"//packages/bazel/src:external.bzl",
"COMMON_ATTRIBUTES",
Expand All @@ -23,6 +22,7 @@ load(
"ts_providers_dict_to_struct",
"tsc_wrapped_tsconfig",
)
load("//packages/bazel/src/ng_module:partial_compilation.bzl", "NgPartialCompilationInfo")

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

if is_perf_requested(ctx):
Expand Down Expand Up @@ -474,6 +475,10 @@ NG_MODULE_ATTRIBUTES = {
executable = True,
cfg = "exec",
),
"is_angular_core_compilation": attr.bool(
default = False,
doc = "Whether this is a compilation of Angular core.",
),
"_partial_compilation_flag": attr.label(
default = "//packages/bazel/src:partial_compilation",
providers = [NgPartialCompilationInfo],
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/private/migrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* package requires for migration schematics.
*/

export {forwardRefResolver} from '../src/ngtsc/annotations';
export {createForwardRefResolver} from '../src/ngtsc/annotations';
export {AbsoluteFsPath} from '../src/ngtsc/file_system';
export {Reference} from '../src/ngtsc/imports';
export {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@ export function resolveEnumValue(
metadata: Map<string, ts.Expression>,
field: string,
enumSymbolName: string,
isCore: boolean,
): number | null {
let resolved: number | null = null;
if (metadata.has(field)) {
const expr = metadata.get(field)!;
const value = evaluator.evaluate(expr) as any;
if (value instanceof EnumValue && isAngularCoreReference(value.enumRef, enumSymbolName)) {
if (
value instanceof EnumValue &&
isAngularCoreReference(value.enumRef, enumSymbolName, isCore)
) {
resolved = value.resolved as number;
} else {
throw createValueHasWrongTypeError(
Expand Down
39 changes: 21 additions & 18 deletions packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,14 @@ export function isAngularCore(decorator: Decorator): decorator is Decorator & {i
return decorator.import !== null && decorator.import.from === CORE_MODULE;
}

export function isAngularCoreReference(reference: Reference, symbolName: string): boolean {
return reference.ownedByModuleGuess === CORE_MODULE && reference.debugName === symbolName;
export function isAngularCoreReference(
reference: Reference,
symbolName: string,
isCore: boolean,
): boolean {
return (
(reference.ownedByModuleGuess === CORE_MODULE || isCore) && reference.debugName === symbolName
);
}

export function findAngularDecorator(
Expand Down Expand Up @@ -225,22 +231,19 @@ export function tryUnwrapForwardRef(
* @param args the arguments to the invocation of the forwardRef expression
* @returns an unwrapped argument if `ref` pointed to forwardRef, or null otherwise
*/
export const forwardRefResolver: ForeignFunctionResolver = (
fn,
callExpr,
resolve,
unresolvable,
) => {
if (!isAngularCoreReference(fn, 'forwardRef') || callExpr.arguments.length !== 1) {
return unresolvable;
}
const expanded = expandForwardRef(callExpr.arguments[0]);
if (expanded !== null) {
return resolve(expanded);
} else {
return unresolvable;
}
};
export function createForwardRefResolver(isCore: boolean): ForeignFunctionResolver {
return (fn, callExpr, resolve, unresolvable) => {
if (!isAngularCoreReference(fn, 'forwardRef', isCore) || callExpr.arguments.length !== 1) {
return unresolvable;
}
const expanded = expandForwardRef(callExpr.arguments[0]);
if (expanded !== null) {
return resolve(expanded);
} else {
return unresolvable;
}
};
}

/**
* Combines an array of resolver functions into a one.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,11 @@ import {
compileInputTransformFields,
compileNgFactoryDefField,
compileResults,
createForwardRefResolver,
extractClassDebugInfo,
extractClassMetadata,
extractSchemas,
findAngularDecorator,
forwardRefResolver,
getDirectiveDiagnostics,
getProviderDiagnostics,
InjectableClassRegistry,
Expand Down Expand Up @@ -492,7 +492,13 @@ export class ComponentDecoratorHandler
} = directiveResult;
const encapsulation: number =
(this.compilationMode !== CompilationMode.LOCAL
? resolveEnumValue(this.evaluator, component, 'encapsulation', 'ViewEncapsulation')
? resolveEnumValue(
this.evaluator,
component,
'encapsulation',
'ViewEncapsulation',
this.isCore,
)
: resolveEncapsulationEnumValueLocally(component.get('encapsulation'))) ??
ViewEncapsulation.Emulated;

Expand All @@ -503,6 +509,7 @@ export class ComponentDecoratorHandler
component,
'changeDetection',
'ChangeDetectionStrategy',
this.isCore,
);
} else if (component.has('changeDetection')) {
changeDetection = new o.WrappedNodeExpr(component.get('changeDetection')!);
Expand Down Expand Up @@ -597,7 +604,7 @@ export class ComponentDecoratorHandler
) {
const importResolvers = combineResolvers([
createModuleWithProvidersResolver(this.reflector, this.isCore),
forwardRefResolver,
createForwardRefResolver(this.isCore),
]);

const importDiagnostics: ts.Diagnostic[] = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
import {
DynamicValue,
EnumValue,
ForeignFunctionResolver,
PartialEvaluator,
ResolvedValue,
traceDynamicValue,
Expand All @@ -65,9 +66,9 @@ import {
import {CompilationMode} from '../../../transform';
import {
assertLocalCompilationUnresolvedConst,
createForwardRefResolver,
createSourceSpan,
createValueHasWrongTypeError,
forwardRefResolver,
getAngularDecorators,
getConstructorDependencies,
isAngularDecorator,
Expand Down Expand Up @@ -373,7 +374,12 @@ export function extractDirectiveMetadata(
const hostDirectives =
rawHostDirectives === null
? null
: extractHostDirectives(rawHostDirectives, evaluator, compilationMode);
: extractHostDirectives(
rawHostDirectives,
evaluator,
compilationMode,
createForwardRefResolver(isCore),
);

if (compilationMode !== CompilationMode.LOCAL && hostDirectives !== null) {
// In global compilation mode where we do type checking, the template type-checker will need to
Expand Down Expand Up @@ -1672,6 +1678,7 @@ function extractHostDirectives(
rawHostDirectives: ts.Expression,
evaluator: PartialEvaluator,
compilationMode: CompilationMode,
forwardRefResolver: ForeignFunctionResolver,
): HostDirectiveMeta[] {
const resolved = evaluator.evaluate(rawHostDirectives, forwardRefResolver);
if (!Array.isArray(resolved)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngtsc/annotations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/// <reference types="node" />

export {
forwardRefResolver,
createForwardRefResolver,
findAngularDecorator,
getAngularDecorators,
isAngularDecorator,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ import {
combineResolvers,
compileDeclareFactory,
compileNgFactoryDefField,
createForwardRefResolver,
createValueHasWrongTypeError,
extractClassMetadata,
extractSchemas,
findAngularDecorator,
forwardRefResolver,
getProviderDiagnostics,
getValidConstructorDependencies,
InjectableClassRegistry,
Expand Down Expand Up @@ -348,6 +348,7 @@ export class NgModuleDecoratorHandler
return {};
}

const forwardRefResolver = createForwardRefResolver(this.isCore);
const moduleResolvers = combineResolvers([
createModuleWithProvidersResolver(this.reflector, this.isCore),
forwardRefResolver,
Expand Down
9 changes: 9 additions & 0 deletions packages/compiler-cli/src/ngtsc/core/api/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,15 @@ export interface InternalOptions {
* Whether to check the event side of two-way bindings.
*/
_checkTwoWayBoundEvents?: boolean;

/**
* Whether this is a compilation of Angular core itself.
*
* By default, we detect this automatically based on the existence of `r3_symbols.ts`
* in the compilation, but there are other test targets within the `core` package that
* import e.g. `Component` relatively and should be detected by the compiler.
*/
_isAngularCoreCompilation?: boolean;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/core/src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1257,7 +1257,8 @@ export class NgCompiler {
}

private makeCompilation(): LazyCompilationState {
const isCore = isAngularCorePackage(this.inputProgram);
const isCore =
this.options._isAngularCoreCompilation ?? isAngularCorePackage(this.inputProgram);

// Note: If this compilation builds `@angular/core`, we always build in full compilation
// mode. Code inside the core package is always compatible with itself, so it does not
Expand Down
32 changes: 19 additions & 13 deletions tools/defaults.bzl
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
"""Re-export of some bazel rules with repository-wide defaults."""

load("@rules_pkg//:pkg.bzl", "pkg_tar")
load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test", _npm_package_bin = "npm_package_bin", _pkg_npm = "pkg_npm")
load("@npm//@bazel/jasmine:index.bzl", _jasmine_node_test = "jasmine_node_test")
load("@npm//@bazel/concatjs:index.bzl", _ts_config = "ts_config", _ts_library = "ts_library")
load("@npm//@bazel/rollup:index.bzl", _rollup_bundle = "rollup_bundle")
load("@npm//@bazel/terser:index.bzl", "terser_minified")
load("@npm//@bazel/protractor:index.bzl", _protractor_web_test_suite = "protractor_web_test_suite")
load("@npm//typescript:index.bzl", "tsc")
load("@npm//@angular/build-tooling/bazel/app-bundling:index.bzl", _app_bundle = "app_bundle")
load("@npm//@angular/build-tooling/bazel/http-server:index.bzl", _http_server = "http_server")
load("@npm//@angular/build-tooling/bazel/karma:index.bzl", _karma_web_test = "karma_web_test", _karma_web_test_suite = "karma_web_test_suite")
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")
load("@npm//@angular/build-tooling/bazel:extract_js_module_output.bzl", "extract_js_module_output")
load("@npm//@angular/build-tooling/bazel:extract_types.bzl", _extract_types = "extract_types")
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")
load("@npm//@angular/build-tooling/bazel/app-bundling:index.bzl", _app_bundle = "app_bundle")
load("@npm//@angular/build-tooling/bazel/esbuild:index.bzl", _esbuild = "esbuild", _esbuild_config = "esbuild_config", _esbuild_esm_bundle = "esbuild_esm_bundle")
load("@npm//@angular/build-tooling/bazel/spec-bundling:spec-entrypoint.bzl", "spec_entrypoint")
load("@npm//@angular/build-tooling/bazel/http-server:index.bzl", _http_server = "http_server")
load("@npm//@angular/build-tooling/bazel/karma:index.bzl", _karma_web_test = "karma_web_test", _karma_web_test_suite = "karma_web_test_suite")
load("@npm//@angular/build-tooling/bazel/spec-bundling:index.bzl", "spec_bundle")
load("@npm//@angular/build-tooling/bazel/spec-bundling:spec-entrypoint.bzl", "spec_entrypoint")
load("@npm//@bazel/concatjs:index.bzl", _ts_config = "ts_config", _ts_library = "ts_library")
load("@npm//@bazel/jasmine:index.bzl", _jasmine_node_test = "jasmine_node_test")
load("@npm//@bazel/protractor:index.bzl", _protractor_web_test_suite = "protractor_web_test_suite")
load("@npm//@bazel/rollup:index.bzl", _rollup_bundle = "rollup_bundle")
load("@npm//@bazel/terser:index.bzl", "terser_minified")
load("@npm//tsec:index.bzl", _tsec_test = "tsec_test")
load("@npm//typescript:index.bzl", "tsc")
load("@rules_pkg//:pkg.bzl", "pkg_tar")
load("//adev/shared-docs/pipeline/api-gen:generate_api_docs.bzl", _generate_api_docs = "generate_api_docs")
load("//packages/bazel:index.bzl", _ng_module = "ng_module", _ng_package = "ng_package")
load("//tools/esm-interop:index.bzl", "enable_esm_node_module_loader", _nodejs_binary = "nodejs_binary", _nodejs_test = "nodejs_test")
load("//adev/shared-docs/pipeline/api-gen:generate_api_docs.bzl", _generate_api_docs = "generate_api_docs")

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

if not entry_point:
entry_point = "public_api.ts"

is_angular_core_compilation = False
if native.package_name().startswith("packages/core"):
is_angular_core_compilation = True

_ng_module(
name = name,
flat_module_out_file = name,
Expand All @@ -184,6 +189,7 @@ def ng_module(name, tsconfig = None, entry_point = None, testonly = False, deps
# `package_name` can be set to allow for the Bazel NodeJS linker to run. This
# allows for resolution of the given target within the `node_modules/`.
package_name = package_name,
is_angular_core_compilation = is_angular_core_compilation,
perf_flag = "//packages/compiler-cli:ng_perf",
**kwargs
)
Expand Down

0 comments on commit a02e270

Please sign in to comment.