Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler-cli): use correct symbol name for default imported symbo… #54495

Closed
wants to merge 1 commit into from

Conversation

JoostK
Copy link
Member

@JoostK JoostK commented Feb 18, 2024

…ls in defer blocks

This commit addresses a problem with PR #53695 that introduced support for default imports, where the actual dynamic import used in the defer loading function continued to use the symbol name, instead of .default for the dynamic import. This issue went unnoticed in the testcase because a proper instance was being generated for the ɵsetClassMetadataAsync function, but not the generated dependency loader function.

Fixes #54491

@JoostK JoostK added target: patch This PR is targeted for the next patch release area: compiler Issues related to `ngc`, Angular's template compiler core: defer Issues related to @defer blocks. labels Feb 18, 2024
@ngbot ngbot bot modified the milestone: Backlog Feb 18, 2024
…ls in defer blocks

This commit addresses a problem with PR angular#53695 that introduced support for default imports,
where the actual dynamic import used in the defer loading function continued to use the
symbol name, instead of `.default` for the dynamic import. This issue went unnoticed in the
testcase because a proper instance was being generated for the `ɵsetClassMetadataAsync` function,
but not the generated dependency loader function.

Fixes angular#54491
@JoostK JoostK force-pushed the compiler/defer-default-import branch from f97e3a1 to da14b1d Compare February 18, 2024 20:07
@JoostK JoostK marked this pull request as ready for review February 18, 2024 20:30
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I guess I messed this up when I was rebasing #53695 after a long pause.

@JoostK JoostK added the action: merge The PR is ready for merge by the caretaker label Feb 19, 2024
@alxhub
Copy link
Member

alxhub commented Feb 20, 2024

This PR was merged into the repository by commit 0c8744c.

alxhub pushed a commit that referenced this pull request Feb 20, 2024
…ls in defer blocks (#54495)

This commit addresses a problem with PR #53695 that introduced support for default imports,
where the actual dynamic import used in the defer loading function continued to use the
symbol name, instead of `.default` for the dynamic import. This issue went unnoticed in the
testcase because a proper instance was being generated for the `ɵsetClassMetadataAsync` function,
but not the generated dependency loader function.

Fixes #54491

PR Close #54495
@alxhub alxhub closed this in 0c8744c Feb 20, 2024
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler core: defer Issues related to @defer blocks. target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Defer blocks export default regression after upgrading to v17.2
3 participants