[WIP] feat: rewrite of ngx-sub-form without inheritance #176
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a major architecture change which is brought without any breaking change 😄! We've split up the code base in 2: Old one and new one. The old one hasn't moved at all but is now deprecated (not removed yet!). You can keep using the old one for a bit and have a smooth/incremental update to use the new API. Few changes that you have to note with the new API: - Only works with Angular 9 or more - The app needs to have Ivy activated (this is because we use `ɵmarkDirty` internally. If it ever gets removed we'll probably have to ask to provide the `ChangeDetectorRef` but we were able to around this for now!) - We got rid of inheritance 🙌 - Form errors on a FormArray are now an object instead of an array. Previously the array contained null values on all the fields without any error. It's now an object containing only the ones with errors and you can access them using the index Please start upgrading to the new API as soon as possible as we stop supporting the old API as of today and will remove it in a near release. This closes #171 for the major architectural changes and also the following issues as a result: - closes #82 - closes #86 - closes #93 - closes #133 - closes #143 - closes #144 - closes #149 - closes #160 - closes #168
maxime1992
added
scope: doc
This was referenced Jun 15, 2020
tyroneneill
reviewed
Jun 15, 2020
| const writeValue$$: ReplaySubject<Nilable<ControlInterface>> = new ReplaySubject(1); | ||
| const registerOnChange$$: ReplaySubject<(formValue: ControlInterface | null) => void> = new ReplaySubject(1); | ||
| const registerOnTouched$$: ReplaySubject<(_: any) => void> = new ReplaySubject(1); | ||
| const setDisabledState$$: ReplaySubject<boolean> = new ReplaySubject(1); |
There was a problem hiding this comment.
This might be better suited to being a BehaviourSubject?
| writeValue$: writeValue$$.asObservable(), | ||
| registerOnChange$: registerOnChange$$.asObservable(), | ||
| registerOnTouched$: registerOnTouched$$.asObservable(), | ||
| setDisabledState$: setDisabledState$$.asObservable(), |
There was a problem hiding this comment.
I think the names need to be changed to highlight these are hooks/listeners? it is weird to listen to a property prefixed with set 👀
|
|
||
| export const patchClassInstance = (componentInstance: any, obj: Object) => { | ||
| Object.entries(obj).forEach(([key, newMethod]) => { | ||
| componentInstance[key] = newMethod; |
There was a problem hiding this comment.
What happens if the instance already has one of the hooks defined? is it on purpose we do not use safelyPatchClassInstance ?
maxime1992
force-pushed
the
feat/rewrite
branch
2 times, most recently
from
9f82ea4 to
4161809
Compare
maxime1992
force-pushed
the
feat/rewrite
branch
from
4161809 to
1e2a3fd
Compare
…orm and ngx-sub-form/new to avoid making a breaking change
maxime1992
force-pushed
the
feat/rewrite
branch
from
9f4d411 to
3db939b
Compare
|
I'm going to close #176 in favor of #188 which is the same of except that I renamed the branch to go around the publishing error with the not supported format for the branch feat/rewrite which I renamed in the new branch to feat-rewrite. @tyroneneill I'll keep your comments in mind before merging and have a look at some point 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commit description for more.