Add isExecutingOnNavigation property into trigger fix #9483

[andrewtelnov]

No description provided.

[SamMousa]

I think it'll work for sure, but do we really need the separation between isOnNavigation and isOnNextPage?

From my perspective there's just 2 scenarios, the onValueChange and onNavigation executions...

I'm worried that adding so many variables makes it hard to maintain.

Also, while you're changing checkTriggers, I think you could add more strict typing :-)

Regardless, if you prefer it like this, this will solve my problem as far as i can tell! <3

[SamMousa]

Suggested change

	const key: any = {};
	const key: Record<string, unknown> = {};

[andrewtelnov]

It is better to be consistent. We use to have any for these scenarios.

Thank you,
Andrew

[andrewtelnov]

I think it'll work for sure, but do we really need the separation between isOnNavigation and isOnNextPage?

From my perspective there's just 2 scenarios, the onValueChange and onNavigation executions...

I can't remove isExecutingOnNextPage. It could break somebodies code. You can use isExecutingOnNavigation. It will be true for both scenarios.

Thank you,
Andrew

[SamMousa]

I can't remove isExecutingOnNextPage. It could break somebodies code. You can use isExecutingOnNavigation. It will be true for both scenarios.

Fully agree, what about deprecating it? So it can be removed some time in the future?

Edit: internally you could even make it a read only property that just mirrors the value of isExeuctingOnNavigation thereby making the backwards compatibility something only appearing inside the trigger, not outside.

[andrewtelnov]

I can't remove isExecutingOnNextPage. It could break somebodies code. You can use isExecutingOnNavigation. It will be true for both scenarios.

Fully agree, what about deprecating it? So it can be removed some time in the future?

Yes, we can do it in the future.

Thank you,
Andrew

[andrewtelnov]

@SamMousa I stopped using isExecutingOnNextPage property.

Thank you,
Andrew