-
Notifications
You must be signed in to change notification settings - Fork 26.3k
feat(core): add support for two-way bindings on dynamically-created components #60342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
kind: 'twoWay', | ||
requiredVars: input[BINDING].requiredVars + output[BINDING].requiredVars, | ||
}, | ||
set target(target: unknown) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was debating whether to make this a setter or change target
to be a setTarget
function. I went with the former, because the only case where it being a function would be useful is two-way bindings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would work on model inputs too I suppose, even though I didn't see any tests for that, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it will. I didn't set up any tests for it, because most of our current tests are using JIT and the new function-based APIs need a TS transform to work correctly in JIT.
…omponents Builds on the changes from angular#60137 to add support for two-way bindings on dynamically-created components. Example usage: ```typescript import {createComponent, signal, twoWayBinding} from '@angular/core'; const value = signal(''); createComponent(MyCheckbox, { bindings: [ twoWayBinding('value', value), ], }); ``` In the example above the value of `MyCheckbox` and the `value` signal will be kept in sync.
03a4eb5
to
64658c5
Compare
expect(dirInstance.value).toBe('dir changed'); | ||
}); | ||
|
||
it('should not bind root component two-way bindings to directives', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a good test to add 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Reviewed-for: public-api
Reviewed-for: fw-general
* @param value Writable signal from which to get the current value and to which to write new | ||
* values. | ||
* | ||
* ### Usage example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is it potentially worth to {@link ...}
to the API that this function is supposed to be used with?
* | ||
* ### Usage example | ||
* In this example we create an instance of the `MyCheckbox` component and bind to its `value` | ||
* input using a two-way binding. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not worth doing, but have we thought about some recommendations for type safety here? e.g. potentially users could do for public model fields
twoWayBinding('bla', mySignal as TargetComponent['modelField'])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for public API; I know per design doc we've decided that we can't reliably support any type safety here
Reviewed-for: public-api
This PR was merged into the repository by commit b154fb3. The changes were merged into the following branches: main |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Builds on the changes from #60137 to add support for two-way bindings on dynamically-created components. Example usage:
In the example above the value of
MyCheckbox
and thevalue
signal will be kept in sync.