feat(ivy): generate ɵɵproperty instructions

[benlesh]

• Adds generation of ɵɵproperty instructions to compiler
• Fixes issues where global state for selected index and active directives were getting out of sync.
• Resolves an issue where selected index was being prematurely set before hooks were being executed.

Related #29881

[benlesh]

Current integration_test failures seem to be related to an issue with incorrect version numbers in Angular CLI. 🤔

[mhevery]

Can you explain whey all of the try-finally blocks are needed?

[kara]

This mapping logic (lines 757-766) is already in mapBindingToInstruction. Reuse here?

instruction = mapBindingToInstruction(inputType)

[benlesh]

mapBindingToInstruction was removed. The way it was being used didn't make sense. For example, it was mapping for BindingType.Animation, but for no reason, because what it was mapping to wasn't even used inside the block for if (inputType === BindingType.Animation) { right below it.

Looking into your comment did reveal a funny bug though, it was still doing else if (instruction) which was always true because instruction is a function declared elsewhere in the module. :)

At the end of the day, the mapBindingToInstruction function was unnecessary indirection that wasn't even universally useful for every BindingType. I axed it in favor of simple conditionals. These are likely to change in upcoming PRs to update generated code for inrepolation too, to be even more like the check above where you see inputType === BindingType.Property && !(value instanceof Interpolation)

[kara]

Thanks for fixing the logic. I would argue that we should still move it to a separate function for readability. This function is incredibly long already, so I think inlining it actually makes the code less readable.

[benlesh]

The problem is that the BindingType does not always map 1:1 with an instruction anymore, and that's actually liable to get more complicated. I agree that the mega function sucks, but perhaps we can refactor it in another PR?

[kara]

I'm not sure I see why it's hard to copy-paste that code out into a different function?

[benlesh]

Can you explain whey all of the try-finally blocks are needed?

@mhevery There are several situations where updating a property causes child views to synchronously render. For example when you update an ngIf property, it may call the template function for an embedded view synchronously. When those templates execute, they reset the selected index to -1 to start with, then they have their own ɵɵselect instructions inside of them which also alter the selected index state. Since the selected index is shared, global state, we need to set it back when we're done, even if there's an error.

[googlebot]

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

[googlebot]

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[mhevery]

LGTM, please resolve comments from other.

[kara]

LGTM, a few more nits and should be good

[angular-automatic-lock-bot]

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.}