Deployed adev-preview for 59e169c to: https://ng-dev-previews-fw--pr-angular-angular-57549-adev-prev-oyzoua3t.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

I love this change, but since the signalification of the primitive values returned by each phase callback is abstracted away from us, this seems to be a bad fit for use cases that require multiple values to be passed from one phase to another. For example:

afterRenderEffect({
  earlyRead: () => {
    const element = this.someElementRef.nativeElement;
    return [element.offsetLeft, element.offsetTop];
  },
  write: ([x, y]) => animateThing(x, y)
})

In this example, I assume that the write phase would run on every render because the earlyRead phase returns a new array each time. Is that correct?

The workaround I use today isn't very ergonomic:

position = signal<[number,number]>([0,0], {equal: arrayDeepEqual});

afterRender({
  earlyRead: () => {
    const element = this.someElementRef().nativeElement;
    this.position.set([element.offsetLeft, element.offsetTop]);
  }
});

// only run the write phase when this.position changes, after accounting for custom equality fn
effect(() => {
  const [x, y] = this.position();
  afterNextRender({ write: () => animateThing(x, y) });
});

It seems to work, but it feels brittle. What do you think?

LGTM overall but adding a cleanup label since:

• is missing public API export
• it has pending comments from other reviewers

@garrettld

I love this change, but since the signalification of the primitive values returned by each phase callback is abstracted away from us, this seems to be a bad fit for use cases that require multiple values to be passed from one phase to another.

Yeah, we saw this too, but I don't know if there is an API that's ideal here. There seem to be two potential design avenues which address the problem:

1. Allowing some configuration of equality function (which users would have to remember to do). This could be specified in the options, or by allowing the signal to be directly created by the user somehow.

2. Increasing the granularity at which we make signals out of the returned object. For example, we could require that phases return an object & make signals out of each property, or explicitly support returning tuples.

Both of these solutions have their downsides. With equality functions, you have to remember to pass them. With more granular return objects, the signal wrapping only works at one layer - if I get creative and return {bounds: {x: 0, y: 3}} then I break things again. I think we're leaning in favor of equality as a more explicit solution.

@alxhub the test CI job is failing with the payload size issue. Could you please update golden file(s)?

This PR was merged into the repository by commit be2e496.

The changes were merged into the following branches: main

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