ionic-framework icon indicating copy to clipboard operation
ionic-framework copied to clipboard

fix(angular): setting props on a signal works

Open liamdebeasi opened this issue 1 year ago • 3 comments

Issue number: resolves #28876


What is the current behavior?

When assigning componentProps as inputs to an Angular component, we do Object.assign. When using the newer Angular Signals API for inputs the value of an input is a function:

myInput = input<string>('foo') // this is a function

The developer accesses the value of myInput in a template by doing myInput() since myInput is a function.

If a developer passes componentProps: { myInput: 'bar' } then the value of myInput is set to this string value, overriding the function. As a result, calling myInput() results in an error because myInput is a string not a function.

What is the new behavior?

  • Angular 14.1 introduced setInput which lets us hand off setting inputs to Angular. This will set input values properly even when using a Signals-based input.

Does this introduce a breaking change?

  • [x] Yes
  • [ ] No

As part of this NavParams has been removed as it is incompatible with the setInput API. The old Object.assign worked to allow devs to get all of the componentProp key value pairs via NavParams even if they are not defined as Inputs. Using setInput will now throw an error, so developers need to create an @Input for each parameter. This means that NavParams has no purpose and can safely be retired in favor of Angular's Input API. Not removing NavParms would make it difficult for us to support new Angular APIs such as this Signals-based input API.

Other information

Dev build: 7.6.2-dev.11706205501.196a5433

liamdebeasi avatar Jan 25 '24 15:01 liamdebeasi

@liamdebeasi is this PR ready for review?

thetaPC avatar Jan 30 '24 17:01 thetaPC

Not yet. This requires a breaking change that I want to run by the team first.

liamdebeasi avatar Jan 30 '24 21:01 liamdebeasi

I'll close for now.

liamdebeasi avatar Jan 30 '24 21:01 liamdebeasi

@brandyscarney Any hope Angular signals might be supported in future versions of Ionic?

ntorrey avatar Apr 17 '24 23:04 ntorrey

The branch is merged into main recently and its name was 8.0, so I guess it will be in there? ^^

muuvmuuv avatar Apr 18 '24 06:04 muuvmuuv

I did not close this PR. It was automatically closed when the feature-8.0 branch was deleted. Liam will have to open a new pull request to get this merged.

brandyscarney avatar Apr 18 '24 20:04 brandyscarney

@brandyscarney does someone has this in their mind, now where liam left?

EinfachHans avatar May 03 '24 10:05 EinfachHans

A new PR has been reopened! I recommend subscribing to it for more updates.

thetaPC avatar May 03 '24 16:05 thetaPC