From 204b91ab484a6774e6a2a1d68a069e7f368162c6 Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Thu, 20 Mar 2025 11:01:02 +0000
Subject: [PATCH] fix(multiple): ensure re-exported module symbols can be
imported
As of recently, we switched our imports from module imports to relative
imports, when inside the same package. This is expected for proper code
splitting, and also for our upcoming `rules_js` migration.
Unfortunately this highlights an issue with the Angular compiler. We
occasionally re-export other entry-point modules from one entry-point
module. This is likely done for convenience, but we should stop doing
that going forward (and likely will naturally resolve this over time
with standalone either way).
The problem is that the Angular compiler, especially with HMR enabled
(i.e. no tree-shaking of imports), will try to generate imports in the
user code to symbols that are indirectly re-exported. This worked before
because the Angular compiler leveraged the "best guessed module", based
on the "last module import" it saw before e.g. discovering the
re-exported `ScrollingModule`; hence it assumed all exports of that
module are available from `@angular/cdk/scrolling`. This assumption no
longer works because the last module import would be e.g. `cdk/overlay`,
which relatively imports from scrolling to re-export the module then.
There are a few options:
- teach the compiler about relative imports inside APF packages with
secondary entry-points. Possible, but won't work for users with older
compiler versions. Maybe a long-term good thing to do; on the other
hand, standalone is the new future.
- switch back to module imports. Not possible and relative imports
should work inside a package IMO.
- re-export the exposed symbols, under a private name. This is the
easiest approach and there also aren't a lot of module re-exports; so
this is a viable approach taken by this commit.
Inside Google, the latter approach is automatically emitted by the
compiler, when everything is built from source; but that's not usable
here; but confirms this approach as being reasonable. Ideally we will
stop re-exporting modules in APF packages, and long-term we start
supporting the proper module guessing with relative imports.
Fixes #30663.
---
integration/module-tests/BUILD.bazel | 27 ++++++
integration/module-tests/find-all-modules.mts | 60 +++++++++++++
integration/module-tests/index.bzl | 24 ++++++
integration/module-tests/index.mts | 86 +++++++++++++++++++
integration/module-tests/tsconfig.json | 8 ++
src/cdk/dialog/dialog-module.ts | 11 +++
src/cdk/drag-drop/drag-drop-module.ts | 6 ++
src/cdk/overlay/overlay-module.ts | 13 +++
src/cdk/scrolling/scrolling-module.ts | 6 ++
9 files changed, 241 insertions(+)
create mode 100644 integration/module-tests/BUILD.bazel
create mode 100644 integration/module-tests/find-all-modules.mts
create mode 100644 integration/module-tests/index.bzl
create mode 100644 integration/module-tests/index.mts
create mode 100644 integration/module-tests/tsconfig.json
diff --git a/integration/module-tests/BUILD.bazel b/integration/module-tests/BUILD.bazel
new file mode 100644
index 000000000000..ae8882384d44
--- /dev/null
+++ b/integration/module-tests/BUILD.bazel
@@ -0,0 +1,27 @@
+load("//tools:defaults2.bzl", "ts_project")
+load("//integration/module-tests:index.bzl", "module_test")
+
+ts_project(
+ name = "test_lib",
+ testonly = True,
+ srcs = glob(["*.mts"]),
+ source_map = False,
+ tsconfig = "tsconfig.json",
+ deps = [
+ "//:node_modules/@angular/compiler-cli",
+ "//:node_modules/@types/node",
+ "//:node_modules/typescript",
+ ],
+)
+
+module_test(
+ name = "test",
+ npm_packages = {
+ "//src/cdk:npm_package": "src/cdk/npm_package",
+ "//src/material:npm_package": "src/material/npm_package",
+ },
+ skipped_entry_points = [
+ # This entry-point is JIT and would fail the AOT test.
+ "@angular/material/dialog/testing",
+ ],
+)
diff --git a/integration/module-tests/find-all-modules.mts b/integration/module-tests/find-all-modules.mts
new file mode 100644
index 000000000000..bc1c8190a4e4
--- /dev/null
+++ b/integration/module-tests/find-all-modules.mts
@@ -0,0 +1,60 @@
+import * as fs from 'node:fs/promises';
+import * as path from 'node:path';
+import * as ts from 'typescript';
+
+export async function findAllEntryPointsAndExportedModules(packagePath: string) {
+ const packageJsonRaw = await fs.readFile(path.join(packagePath, 'package.json'), 'utf8');
+ const packageJson = JSON.parse(packageJsonRaw) as {
+ name: string;
+ exports: Record<string, Record<string, string>>;
+ };
+ const tasks: Promise<{importPath: string; symbolName: string}[]>[] = [];
+
+ for (const [subpath, conditions] of Object.entries(packageJson.exports)) {
+ if (conditions.types === undefined) {
+ continue;
+ }
+
+ tasks.push(
+ (async () => {
+ const dtsFile = path.join(packagePath, conditions.types);
+ const dtsBundleFile = ts.createSourceFile(
+ dtsFile,
+ await fs.readFile(dtsFile, 'utf8'),
+ ts.ScriptTarget.ESNext,
+ false,
+ );
+
+ return scanExportsForModules(dtsBundleFile).map(e => ({
+ importPath: path.posix.join(packageJson.name, subpath),
+ symbolName: e,
+ }));
+ })(),
+ );
+ }
+
+ const moduleExports = (await Promise.all(tasks)).flat();
+
+ return {
+ name: packageJson.name,
+ packagePath,
+ moduleExports,
+ };
+}
+
+function scanExportsForModules(sf: ts.SourceFile): string[] {
+ const moduleExports: string[] = [];
+ const visit = (node: ts.Node) => {
+ if (ts.isExportDeclaration(node) && ts.isNamedExports(node.exportClause)) {
+ moduleExports.push(
+ ...node.exportClause.elements
+ .filter(e => e.name.text.endsWith('Module'))
+ .map(e => e.name.text),
+ );
+ }
+ };
+
+ ts.forEachChild(sf, visit);
+
+ return moduleExports;
+}
diff --git a/integration/module-tests/index.bzl b/integration/module-tests/index.bzl
new file mode 100644
index 000000000000..0db7fe80206a
--- /dev/null
+++ b/integration/module-tests/index.bzl
@@ -0,0 +1,24 @@
+load("@aspect_rules_js//js:defs.bzl", "js_test")
+load("@bazel_skylib//rules:write_file.bzl", "write_file")
+
+def module_test(name, npm_packages, skipped_entry_points = [], additional_deps = []):
+ write_file(
+ name = "%s_config" % name,
+ out = "%s_config.json" % name,
+ content = [json.encode({
+ "packages": [pkg[1] for pkg in npm_packages.items()],
+ "skipEntryPoints": skipped_entry_points,
+ })],
+ )
+
+ js_test(
+ name = "test",
+ data = [
+ ":%s_config" % name,
+ "//integration/module-tests:test_lib_rjs",
+ "//:node_modules/@angular/common",
+ "//:node_modules/@angular/core",
+ ] + additional_deps + [pkg[0] for pkg in npm_packages.items()],
+ entry_point = ":index.mjs",
+ fixed_args = ["$(rootpath :%s_config)" % name],
+ )
diff --git a/integration/module-tests/index.mts b/integration/module-tests/index.mts
new file mode 100644
index 000000000000..f5a587483e8b
--- /dev/null
+++ b/integration/module-tests/index.mts
@@ -0,0 +1,86 @@
+import {performCompilation} from '@angular/compiler-cli';
+import * as fs from 'fs/promises';
+import * as path from 'path';
+import * as os from 'os';
+import ts from 'typescript';
+import {findAllEntryPointsAndExportedModules} from './find-all-modules.mjs';
+
+async function main() {
+ const [configPath] = process.argv.slice(2);
+ const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ng-module-test-'));
+ const config = JSON.parse(await fs.readFile(configPath, 'utf8')) as {
+ packages: string[];
+ skipEntryPoints: string[];
+ };
+
+ const packages = await Promise.all(
+ config.packages.map(pkgPath => findAllEntryPointsAndExportedModules(pkgPath)),
+ );
+
+ const exports = packages
+ .map(p => p.moduleExports)
+ .flat()
+ .filter(e => !config.skipEntryPoints.includes(e.importPath));
+
+ const testFile = `
+ import {NgModule, Component} from '@angular/core';
+ ${exports.map(e => `import {${e.symbolName}} from '${e.importPath}';`).join('\n')}
+
+ @NgModule({
+ exports: [
+ ${exports.map(e => e.symbolName).join(', ')}
+ ]
+ })
+ export class TestModule {}
+
+ @Component({imports: [TestModule], template: ''})
+ export class TestComponent {}
+ `;
+
+ await fs.writeFile(path.join(tmpDir, 'test.ts'), testFile);
+
+ // Prepare node modules to resolve e.g. `@angular/core`
+ await fs.symlink(path.resolve('./node_modules'), path.join(tmpDir, 'node_modules'));
+ // Prepare node modules to resolve e.g. `@angular/cdk`. This is possible
+ // as we are inside the sandbox, inside our test runfiles directory.
+ for (const {packagePath, name} of packages) {
+ await fs.symlink(path.resolve(packagePath), `./node_modules/${name}`);
+ }
+
+ const result = performCompilation({
+ options: {
+ rootDir: tmpDir,
+ skipLibCheck: true,
+ noEmit: true,
+ module: ts.ModuleKind.ESNext,
+ moduleResolution: ts.ModuleResolutionKind.Bundler,
+ strictTemplates: true,
+ preserveSymlinks: true,
+ strict: true,
+ // Note: HMR is needed as it will disable the Angular compiler's tree-shaking of used
+ // directives/components. This is critical for this test as it allows us to simply all
+ // modules and automatically validate that all symbols are reachable/importable.
+ _enableHmr: true,
+ },
+ rootNames: [path.join(tmpDir, 'test.ts')],
+ });
+
+ console.error(
+ ts.formatDiagnosticsWithColorAndContext(result.diagnostics, {
+ getCanonicalFileName: f => f,
+ getCurrentDirectory: () => '/',
+ getNewLine: () => '\n',
+ }),
+ );
+
+ await fs.rm(tmpDir, {recursive: true, force: true, maxRetries: 2});
+
+ if (result.diagnostics.length > 0) {
+ process.exitCode = 1;
+ }
+}
+
+main().catch(e => {
+ console.error('Error', e);
+ process.exitCode = 1;
+});
diff --git a/integration/module-tests/tsconfig.json b/integration/module-tests/tsconfig.json
new file mode 100644
index 000000000000..cb1f72085b50
--- /dev/null
+++ b/integration/module-tests/tsconfig.json
@@ -0,0 +1,8 @@
+{
+ "compilerOptions": {
+ "module": "ESNext",
+ "moduleResolution": "bundler",
+ "declaration": true,
+ "types": ["node"]
+ }
+}
diff --git a/src/cdk/dialog/dialog-module.ts b/src/cdk/dialog/dialog-module.ts
index 4c800bc30304..bda86b4c4698 100644
--- a/src/cdk/dialog/dialog-module.ts
+++ b/src/cdk/dialog/dialog-module.ts
@@ -24,3 +24,14 @@ import {CdkDialogContainer} from './dialog-container';
providers: [Dialog],
})
export class DialogModule {}
+
+// Re-export needed by the Angular compiler.
+// See: https://github.com/angular/components/issues/30663.
+// Note: These exports need to be stable and shouldn't be renamed unnecessarily because
+// consuming libraries might have references to them in their own partial compilation output.
+export {
+ CdkPortal as ɵɵCdkPortal,
+ CdkPortalOutlet as ɵɵCdkPortalOutlet,
+ TemplatePortalDirective as ɵɵTemplatePortalDirective,
+ PortalHostDirective as ɵɵPortalHostDirective,
+} from '../portal';
diff --git a/src/cdk/drag-drop/drag-drop-module.ts b/src/cdk/drag-drop/drag-drop-module.ts
index 4452a0eb50c3..58032b263cc6 100644
--- a/src/cdk/drag-drop/drag-drop-module.ts
+++ b/src/cdk/drag-drop/drag-drop-module.ts
@@ -31,3 +31,9 @@ const DRAG_DROP_DIRECTIVES = [
providers: [DragDrop],
})
export class DragDropModule {}
+
+// Re-export needed by the Angular compiler.
+// See: https://github.com/angular/components/issues/30663.
+// Note: These exports need to be stable and shouldn't be renamed unnecessarily because
+// consuming libraries might have references to them in their own partial compilation output.
+export {CdkScrollable as ɵɵCdkScrollable} from '../scrolling';
diff --git a/src/cdk/overlay/overlay-module.ts b/src/cdk/overlay/overlay-module.ts
index 0b5721275ec2..fbdc64f710a2 100644
--- a/src/cdk/overlay/overlay-module.ts
+++ b/src/cdk/overlay/overlay-module.ts
@@ -23,3 +23,16 @@ import {
providers: [Overlay, CDK_CONNECTED_OVERLAY_SCROLL_STRATEGY_PROVIDER],
})
export class OverlayModule {}
+
+// Re-export needed by the Angular compiler.
+// See: https://github.com/angular/components/issues/30663.
+// Note: These exports need to be stable and shouldn't be renamed unnecessarily because
+// consuming libraries might have references to them in their own partial compilation output.
+export {
+ CdkScrollableModule as ɵɵCdkScrollableModule,
+ CdkFixedSizeVirtualScroll as ɵɵCdkFixedSizeVirtualScroll,
+ CdkVirtualForOf as ɵɵCdkVirtualForOf,
+ CdkVirtualScrollViewport as ɵɵCdkVirtualScrollViewport,
+ CdkVirtualScrollableWindow as ɵɵCdkVirtualScrollableWindow,
+ CdkVirtualScrollableElement as ɵɵCdkVirtualScrollableElement,
+} from '../scrolling';
diff --git a/src/cdk/scrolling/scrolling-module.ts b/src/cdk/scrolling/scrolling-module.ts
index 276aa83a9121..d30f03537aa5 100644
--- a/src/cdk/scrolling/scrolling-module.ts
+++ b/src/cdk/scrolling/scrolling-module.ts
@@ -45,3 +45,9 @@ export class CdkScrollableModule {}
],
})
export class ScrollingModule {}
+
+// Re-export needed by the Angular compiler.
+// See: https://github.com/angular/components/issues/30663.
+// Note: These exports need to be stable and shouldn't be renamed unnecessarily because
+// consuming libraries might have references to them in their own partial compilation output.
+export {Dir as ɵɵDir} from '../bidi';