Skip to content

Commit f129c5c

Browse files
devversiondevversion
committed
fix(multiple): ensure re-exports module symbols can be re-exported
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. Fixes #30663.
1 parent 1b4cae7 commit f129c5c

File tree

9 files changed

+230
-0
lines changed

9 files changed

+230
-0
lines changed

‎integration/module-tests/BUILD.bazel

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
load("//tools:defaults2.bzl", "ts_project")
2+
load("//integration/module-tests:index.bzl", "module_test")
3+
4+
ts_project(
5+
name = "test_lib",
6+
testonly = True,
7+
srcs = glob(["*.mts"]),
8+
source_map = False,
9+
tsconfig = "tsconfig.json",
10+
deps = [
11+
"//:node_modules/@angular/compiler-cli",
12+
"//:node_modules/@types/node",
13+
"//:node_modules/typescript",
14+
],
15+
)
16+
17+
module_test(
18+
name = "test",
19+
npm_packages = {
20+
"//src/cdk:npm_package": "src/cdk/npm_package",
21+
"//src/material:npm_package": "src/material/npm_package",
22+
},
23+
skipped_entry_points = [
24+
# This entry-point is JIT and would fail the AOT test.
25+
"@angular/material/dialog/testing",
26+
],
27+
)

‎integration/module-tests/find-all-modules.mts

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import * as fs from 'node:fs/promises';
2+
import * as path from 'node:path';
3+
import * as ts from 'typescript';
4+
5+
export async function findAllEntryPointsAndExportedModules(packagePath: string) {
6+
const packageJsonRaw = await fs.readFile(path.join(packagePath, 'package.json'), 'utf8');
7+
const packageJson = JSON.parse(packageJsonRaw) as {
8+
name: string;
9+
exports: Record<string, Record<string, string>>;
10+
};
11+
const tasks: Promise<{importPath: string; symbolName: string}[]>[] = [];
12+
13+
for (const [subpath, conditions] of Object.entries(packageJson.exports)) {
14+
if (conditions.types === undefined) {
15+
continue;
16+
}
17+
18+
tasks.push(
19+
(async () => {
20+
const dtsFile = path.join(packagePath, conditions.types);
21+
const dtsBundleFile = ts.createSourceFile(
22+
dtsFile,
23+
await fs.readFile(dtsFile, 'utf8'),
24+
ts.ScriptTarget.ESNext,
25+
false,
26+
);
27+
28+
return scanExportsForModules(dtsBundleFile).map(e => ({
29+
importPath: path.posix.join(packageJson.name, subpath),
30+
symbolName: e,
31+
}));
32+
})(),
33+
);
34+
}
35+
36+
const moduleExports = (await Promise.all(tasks)).flat();
37+
38+
return {
39+
name: packageJson.name,
40+
packagePath,
41+
moduleExports,
42+
};
43+
}
44+
45+
function scanExportsForModules(sf: ts.SourceFile): string[] {
46+
const moduleExports: string[] = [];
47+
const visit = (node: ts.Node) => {
48+
if (ts.isExportDeclaration(node) && ts.isNamedExports(node.exportClause)) {
49+
moduleExports.push(
50+
...node.exportClause.elements
51+
.filter(e => e.name.text.endsWith('Module'))
52+
.map(e => e.name.text),
53+
);
54+
}
55+
};
56+
57+
ts.forEachChild(sf, visit);
58+
59+
return moduleExports;
60+
}

‎integration/module-tests/index.bzl

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
load("@aspect_rules_js//js:defs.bzl", "js_test")
2+
load("@bazel_skylib//rules:write_file.bzl", "write_file")
3+
4+
def module_test(name, npm_packages, skipped_entry_points = [], additional_deps = []):
5+
write_file(
6+
name = "%s_config" % name,
7+
out = "%s_config.json" % name,
8+
content = [json.encode({
9+
"packages": [pkg[1] for pkg in npm_packages.items()],
10+
"skipEntryPoints": skipped_entry_points,
11+
})],
12+
)
13+
14+
js_test(
15+
name = "test",
16+
data = [
17+
":%s_config" % name,
18+
"//integration/module-tests:test_lib_rjs",
19+
"//:node_modules/@angular/common",
20+
"//:node_modules/@angular/core",
21+
] + additional_deps + [pkg[0] for pkg in npm_packages.items()],
22+
entry_point = ":index.mjs",
23+
fixed_args = ["$(rootpath :%s_config)" % name],
24+
)

‎integration/module-tests/index.mts

+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import {performCompilation} from '@angular/compiler-cli';
2+
import * as fs from 'fs/promises';
3+
import * as path from 'path';
4+
import * as os from 'os';
5+
import ts from 'typescript';
6+
import {findAllEntryPointsAndExportedModules} from './find-all-modules.mjs';
7+
8+
async function main() {
9+
const [configPath] = process.argv.slice(2);
10+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ng-module-test-'));
11+
const config = JSON.parse(await fs.readFile(configPath, 'utf8')) as {
12+
packages: string[];
13+
skipEntryPoints: string[];
14+
};
15+
16+
const packages = await Promise.all(
17+
config.packages.map(pkgPath => findAllEntryPointsAndExportedModules(pkgPath)),
18+
);
19+
20+
const exports = packages
21+
.map(p => p.moduleExports)
22+
.flat()
23+
.filter(e => !config.skipEntryPoints.includes(e.importPath));
24+
25+
const testFile = `
26+
import {NgModule, Component} from '@angular/core';
27+
${exports.map(e => `import {${e.symbolName}} from '${e.importPath}';`).join('\n')}
28+
29+
@NgModule({
30+
exports: [
31+
${exports.map(e => e.symbolName).join(', ')}
32+
]
33+
})
34+
export class TestModule {}
35+
36+
@Component({imports: [TestModule], template: ''})
37+
export class TestComponent {}
38+
`;
39+
40+
await fs.writeFile(path.join(tmpDir, 'test.ts'), testFile);
41+
42+
// Prepare node modules to resolve e.g. `@angular/core`
43+
await fs.symlink(path.resolve('./node_modules'), path.join(tmpDir, 'node_modules'));
44+
// Prepare node modules to resolve e.g. `@angular/cdk`. This is possible
45+
// as we are inside the sandbox, inside our test runfiles directory.
46+
for (const {packagePath, name} of packages) {
47+
await fs.symlink(path.resolve(packagePath), `./node_modules/${name}`);
48+
}
49+
50+
const result = performCompilation({
51+
options: {
52+
rootDir: tmpDir,
53+
skipLibCheck: true,
54+
noEmit: true,
55+
module: ts.ModuleKind.ESNext,
56+
moduleResolution: ts.ModuleResolutionKind.Bundler,
57+
strictTemplates: true,
58+
preserveSymlinks: true,
59+
strict: true,
60+
_enableHmr: true,
61+
},
62+
rootNames: [path.join(tmpDir, 'test.ts')],
63+
});
64+
65+
console.error(
66+
ts.formatDiagnosticsWithColorAndContext(result.diagnostics, {
67+
getCanonicalFileName: f => f,
68+
getCurrentDirectory: () => '/',
69+
getNewLine: () => '\n',
70+
}),
71+
);
72+
73+
await fs.rm(tmpDir, {recursive: true, force: true, maxRetries: 2});
74+
75+
if (result.diagnostics.length > 0) {
76+
process.exitCode = 1;
77+
}
78+
}
79+
80+
main().catch(e => {
81+
console.error('Error', e);
82+
process.exitCode = 1;
83+
});

‎integration/module-tests/tsconfig.json

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"compilerOptions": {
3+
"module": "ESNext",
4+
"moduleResolution": "bundler",
5+
"declaration": true,
6+
"types": ["node"]
7+
}
8+
}

‎src/cdk/dialog/dialog-module.ts

+9
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,12 @@ import {CdkDialogContainer} from './dialog-container';
2424
providers: [Dialog],
2525
})
2626
export class DialogModule {}
27+
28+
// Re-export needed by the Angular compiler.
29+
// See: https://github.com/angular/components/issues/30663.
30+
export {
31+
CdkPortal as ɵɵ1,
32+
CdkPortalOutlet as ɵɵ2,
33+
TemplatePortalDirective as ɵɵ3,
34+
PortalHostDirective as ɵɵ4,
35+
} from '../portal';

‎src/cdk/drag-drop/drag-drop-module.ts

+4
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,7 @@ const DRAG_DROP_DIRECTIVES = [
3131
providers: [DragDrop],
3232
})
3333
export class DragDropModule {}
34+
35+
// Re-export needed by the Angular compiler.
36+
// See: https://github.com/angular/components/issues/30663.
37+
export {CdkScrollable as ɵɵ1} from '../scrolling';

‎src/cdk/overlay/overlay-module.ts

+11
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,14 @@ import {
2323
providers: [Overlay, CDK_CONNECTED_OVERLAY_SCROLL_STRATEGY_PROVIDER],
2424
})
2525
export class OverlayModule {}
26+
27+
// Re-export needed by the Angular compiler.
28+
// See: https://github.com/angular/components/issues/30663.
29+
export {
30+
CdkScrollableModule as ɵɵ1,
31+
CdkFixedSizeVirtualScroll as ɵɵ2,
32+
CdkVirtualForOf as ɵɵ3,
33+
CdkVirtualScrollViewport as ɵɵ4,
34+
CdkVirtualScrollableWindow as ɵɵ5,
35+
CdkVirtualScrollableElement as ɵɵ6,
36+
} from '../scrolling';

‎src/cdk/scrolling/scrolling-module.ts

+4
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,7 @@ export class CdkScrollableModule {}
4545
],
4646
})
4747
export class ScrollingModule {}
48+
49+
// Re-export needed by the Angular compiler.
50+
// See: https://github.com/angular/components/issues/30663.
51+
export {Dir as ɵɵ1} from '../bidi';

0 commit comments

Comments
 (0)