Skip to content

fix(compiler-cli): handle forwardRef in imports of standalone component#45869

Closed
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:standalone-import-forward-ref
Closed

fix(compiler-cli): handle forwardRef in imports of standalone component#45869
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:standalone-import-forward-ref

Conversation

@crisbeto
Copy link
Member

@crisbeto crisbeto commented May 4, 2022

Fixes that the compiler wasn't resolving forwardRef values when they're used in the imports of a standalone component.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: compiler Issues related to `ngc`, Angular's template compiler labels May 4, 2022
@ngbot ngbot bot added this to the Backlog milestone May 4, 2022
@pullapprove pullapprove bot requested a review from JoostK May 4, 2022 07:36
@crisbeto crisbeto added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels May 4, 2022
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

I think it would be worthwhile to capture this in a compliance test, as we'd then get a golden file for the partial compilation output and a round-trip with the linker.

Choose a reason for hiding this comment

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

nit: wonder what is going on with the formatting in here? Did a "badly" formatted file end up in the repo? Or is your formatter config off?

Generally speaking I would love to avoid unnecessary diffs (harder to review, potential for a merge conflict etc. etc.)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's because we aren't enforcing trailing whitespaces. I have a setting in my IDE to trim them automatically.

Copy link
Contributor

Choose a reason for hiding this comment

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

As you should per .editorconfig: https://github.com/angular/angular/blob/main/.editorconfig#L11

I've tried to add a linting rule years ago for this and line ending at the end of the file that keeps changing on every file depending on who edits it and it's been annoying for years, but at the time, vicb told me that it's better to allow people do their own thing. I fail to see why, but I'm not a maintainer, ofc

Copy link
Member

Choose a reason for hiding this comment

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

Technically your editor is in the wrong here, as these are multi-line template strings and thus trailing whitespace on a line is load-bearing (it's part of the string contents), definitely not spurious.

Fixes that the compiler wasn't resolving `forwardRef` values when they're used in the `imports` of a standalone component.
@crisbeto crisbeto force-pushed the standalone-import-forward-ref branch from 4a43f68 to b7c40dd Compare May 4, 2022 09:29
@crisbeto
Copy link
Member Author

crisbeto commented May 4, 2022

I've added a compliance test.

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 4, 2022
@dylhunn
Copy link
Contributor

dylhunn commented May 4, 2022

This PR was merged into the repository by commit 32c625d.

@dylhunn dylhunn closed this in 32c625d May 4, 2022
@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 Jun 4, 2022
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 target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants