fix(compiler): bind setClassMetadataAsync parameters to local symbol names - #70108
Draft
mattrbeck wants to merge 1 commit into
Draft
fix(compiler): bind setClassMetadataAsync parameters to local symbol names#70108mattrbeck wants to merge 1 commit into
mattrbeck wants to merge 1 commit into
Conversation
…names
The callback wrapped by `setClassMetadataAsync` replays the component's original decorator, so it
refers to each deferred dependency by the name that the declaring file binds it to. Its parameters
were instead named after the symbols' exported names, because `R3DeferPerComponentDependency`
carried a single `symbolName` that was used both for the `m.<symbolName>` dereference inside the
dynamic import and for the parameter list. Those two uses disagree as soon as a dependency is
imported under an alias:
```ts
import {Dep as AliasedDep} from './dep';
// ...
setClassMetadataAsync(MyApp,
() => [import('./dep').then(m => m.Dep)],
Dep => setClassMetadata(MyApp, [{..., imports: [AliasedDep]}], null, null));
```
The static import is removed in favour of the dynamic one, so nothing binds `AliasedDep` and the
callback throws a `ReferenceError` in dev mode once the defer block loads. Two dependencies aliased
from modules that export the same name were worse still, producing a duplicate parameter list such
as `(Dep, Dep) => ...` which is a syntax error under strict mode.
`R3DeferPerComponentDependency` now carries the local name alongside the exported one. The callback
parameters bind the local name, while the dynamic import keeps dereferencing the exported name, so
the loader itself is unchanged. Local names are necessarily unique within a file, which resolves the
duplicate parameter case as well. When a dependency is not aliased the two names coincide and the
generated output is identical to before. The same applies to the `resolveMetadata` callback of the
partial declaration, and therefore to its linked output.
`removeDeferrableTypesFromComponentDecorator` is switched over for the same reason: it matches
identifiers appearing inside the decorator, which are the local ones, so aliased references were
previously not detached from the import that gets deleted.
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
1 participant
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.
When
@deferblocks are compiled in dev mode,setClassMetadataAsync(and partial declarationresolveMetadata) wraps a callback that replays the component's original decorator metadata. This replayed decorator refers to any deferred dependencies using the local identifiers bound in the declaring file.Previously,
R3DeferPerComponentDependencytracked a singlesymbolNamethat was used both for dereferencing the exported property on the dynamic import (m => m.Dep) and as the parameter name in the metadata callback:Because the static import is stripped in favor of the dynamic loader,
AliasedDepremains unbound in the callback scope, causing a runtimeReferenceErroras soon as the defer block resolves. Even worse, if a component imports dependencies from two separate modules that happen to share the same export name under different local aliases (e.g.import {Dep as DepA} from './a'; import {Dep as DepB} from './b'), both parameters were generated usingDep, yielding duplicate parameter names(Dep, Dep) => ...which is a strict-mode syntax error.In addition,
removeDeferrableTypesFromComponentDecoratorrelied on the exportedsymbolNamewhen stripping identifiers from the component decorator node, which failed to match aliased identifiers.Solution
This PR updates
R3DeferPerComponentDependency(and internal resolution tracking) to distinguish betweensymbolName(the module export key) andlocalSymbolName(the local binding identifier):symbolName(m => m[symbolName]), leaving the dynamic loader mechanism unchanged.setClassMetadataAsyncandresolveMetadata) now bind parameters usinglocalSymbolName.removeDeferrableTypesFromComponentDecorator) matches againstlocalSymbolNameto correctly remove aliased imports from original decorators.Because local identifiers are inherently unique within a file scope, parameter name collisions like
(Dep, Dep) => ...are naturally prevented. For standard, non-aliased imports wherelocalSymbolName === symbolName, the emitted code is completely unchanged, making this a safe, additive fix. Compliance test cases have been added to cover both single aliased deferred dependencies and multi-dependency export collisions.