Repository navigation
fix(@ngtools/webpack): elide non-value aliases and remaining type-only AST constructs - #34594
Merged
alan-agius4 merged 2 commits intoOct 30, 2026
Merged
Conversation
There was a problem hiding this comment.
Code Review
This pull request refines import elision and bootstrap replacement logic in Angular build tools to prevent over-elision of imports, particularly handling type nodes, ambient statements, and unresolved modules more robustly. The feedback highlights two key areas for improvement: optimizing AST traversal performance during node removal by targeting only import declarations, and enhancing code robustness by explicitly parenthesizing and comparing bitwise operations to avoid operator precedence issues.
…y AST constructs Follow up to angular#34582 to cover remaining pure-type import elision cases and partial specifier removal in `replaceBootstrap`: - Elide imported alias symbols that do not resolve to a runtime value (or traverse a type-only alias declaration), handling non-`type` interface re-exports (`export { Foo }`) and interface parameter types under `emitDecoratorMetadata`. - Only rewrite a partially elided import declaration when a runtime value import must be removed or when the declaration was already replaced by a previous transformer. TypeScript skips its own import elision for declarations replaced by a transformer, so leaving other declarations untouched avoids retaining specifiers that TypeScript would have otherwise elided (such as inlined `const enum` imports). - Skip all non-`ExpressionWithTypeArguments` `TypeNode`s when `emitDecoratorMetadata` is disabled, and unconditionally skip `TypeQueryNode` (`typeof X`), `ImportTypeNode`, `IndexSignatureDeclaration`, ambient `declare` statements, and external `export ... from '...'` declarations. - Recursively visit child AST nodes when applying `elideImports` removals in `replaceBootstrap` so individual `ImportSpecifier`s are removed when `platformBrowserDynamic` shares an import with other used runtime exports.
…AST constructs Follow up to angular#34582 to apply the same import elision fixes made to `@ngtools/webpack` to the `replaceBootstrap` transformer used by the application builder: - Elide imported alias symbols that do not resolve to a runtime value (or traverse a type-only alias declaration), handling non-`type` interface re-exports (`export { Foo }`) and interface parameter types under `emitDecoratorMetadata`. - Only rewrite a partially elided import declaration when a runtime value import must be removed or when the declaration was already replaced by a previous transformer, so TypeScript's own import elision continues to apply to all other declarations. - Skip all non-`ExpressionWithTypeArguments` `TypeNode`s when `emitDecoratorMetadata` is disabled, and unconditionally skip `TypeQueryNode` (`typeof X`), `ImportTypeNode`, `IndexSignatureDeclaration`, ambient `declare` statements, and external `export ... from '...'` declarations. - Recursively visit child AST nodes when applying import removals in `replaceBootstrap` so individual `ImportSpecifier`s are removed when `platformBrowserDynamic` shares an import with other used runtime exports.
clydin
force-pushed
the
fix/elide-imports-type-edge-cases
branch
from
October 29, 2026 21:05
265ef0d to
19752d3
Compare
alan-agius4
approved these changes
Oct 30, 2026
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow up to #34582 to cover remaining pure-type import elision cases and
partial specifier removal in
replaceBootstrap:traverse a type-only alias declaration), handling non-
typeinterfacere-exports (
export { Foo }) and interface parameter types underemitDecoratorMetadata.import must be removed or when the declaration was already replaced by a
previous transformer. TypeScript skips its own import elision for
declarations replaced by a transformer, so leaving other declarations
untouched avoids retaining specifiers that TypeScript would have otherwise
elided (such as inlined
const enumimports).ExpressionWithTypeArgumentsTypeNodes whenemitDecoratorMetadatais disabled, and unconditionally skipTypeQueryNode(typeof X),ImportTypeNode,IndexSignatureDeclaration, ambientdeclarestatements, and externalexport ... from '...'declarations.elideImportsremovals inreplaceBootstrapso individualImportSpecifiers are removed whenplatformBrowserDynamicshares an import with other used runtime exports.