Skip to content

fix(core) Resolve forwardRef declarations for render3 jit#45742

Closed
pauldraper wants to merge 1 commit intoangular:mainfrom
pauldraper:pauldraper/forwardref
Closed

fix(core) Resolve forwardRef declarations for render3 jit#45742
pauldraper wants to merge 1 commit intoangular:mainfrom
pauldraper:pauldraper/forwardref

Conversation

@pauldraper
Copy link
Contributor

Resolves #45741

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

NgModule.declarations supports forwardRef for AOT but not JIT

Issue Number: #45741

What is the new behavior?

NgModule.declarations supports forwardRef for JIT

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@pullapprove pullapprove bot requested a review from atscott April 24, 2022 04:25
@pauldraper pauldraper changed the title Resolve forwardRef delcarations for render3 jit fix(core) Resolve forwardRef delcarations for render3 jit Apr 24, 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.

Thanks for the fix! Could you please add a test for this case to prevent regressions (e.g. here) and change the commit message to be prefixed with fix(core): (and you can drop render3 as Ivy is the only remaining renderer).

@pauldraper
Copy link
Contributor Author

pauldraper commented Apr 24, 2022

and you can drop render3 as Ivy is the only remaining renderer

An odd thing to say, considering this is a change to packages/core/src/render3/jit/module.ts

But names are hard :)

@atscott atscott added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: core Issues related to the framework runtime labels Apr 26, 2022
@ngbot ngbot bot modified the milestone: Backlog Apr 26, 2022
@AndrewKushnir AndrewKushnir added the target: patch This PR is targeted for the next patch release label Apr 26, 2022
@atscott
Copy link
Contributor

atscott commented May 16, 2022

Hi @pauldraper, could you please address Joost's change request?

@jessicajaniuk jessicajaniuk changed the title fix(core) Resolve forwardRef delcarations for render3 jit fix(core) Resolve forwardRef declarations for render3 jit May 23, 2022
@jessicajaniuk
Copy link
Contributor

Closing as there's been no activity here. Feel free to open a new pull request with the suggested fixes if you'd like this to land. Thanks!

@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 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NgModule.declarations forwardRef doesn't work for JIT

5 participants