Skip to content

Conversation

@sheikalthaf
Copy link
Contributor

use signal input for isHydrationEnabled so that we can enable OnPush CD to the component, also testcases fails if we convert this to use signal inputs.

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?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

use signal input for isHydrationEnabled so that we can enable OnPush CD to the component, also testcases fails if we convert this to use signal inputs.
@pullapprove pullapprove bot requested a review from devversion September 5, 2024 04:26
@ngbot ngbot bot added this to the Backlog milestone Sep 5, 2024
@sheikalthaf
Copy link
Contributor Author

@devversion as we discussed previously regarding the signal input failing in testcases #56998 (comment) . I created a separate PR for the issue, also attaching the screenshot of the error
image

@devversion
Copy link
Member

Thank you @sheikalthaf. I will check this out soon! appreciate it

@devversion
Copy link
Member

I've looked into it a bit. Thanks for the PR. that's great for debugging.

The issue is:

  • The application code is compiled AOT — that's good!
  • The test code is not compiled with AOT — that's generally okay, but NOT here with Bazel as the Angular JIT transforms don't run, so input() etc. are not detected and break.

Fixing the second point is easy, just use ng_module. But then, we still see failures. That is because the test uses TestBed.overrideComponent! Seems like this is not working well with signal inputs, or other non-decorator APIs.

That is because the setClassMetadata calls (generated for JIT) are used to re-compile the component with the overrides — but there is no decorator for the input() calls, hence this metadata is lost.

Options I could see:

  • Smartly merging directly with the original component metadata in TestBed.
    • Will be hard to detect output as it's non-distinguishable from decorator outputs.
    • I guess, we could fully re-use inputs, queries etc. metadata instead of relying on prop decorators..?
  • Adding synthetic prop decorators to setClassMetadata.
    • Would be great to re-use the JIT transforms.. but those need an import manager; so needs a bit of refactoring!

@devversion
Copy link
Member

Let's close this in favor of a tracking issue. #57944. Thanks again!

@devversion devversion closed this Sep 24, 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 Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants