Skip to content
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

Make sure that *RootFormComponent uses the @DataInput decorator #149

Closed
maxime1992 opened this issue Feb 28, 2020 · 6 comments · Fixed by #188
Closed

Make sure that *RootFormComponent uses the @DataInput decorator #149

maxime1992 opened this issue Feb 28, 2020 · 6 comments · Fixed by #188
Assignees
Labels
effort-1: minutes Will only take a few minutes to fix/create flag: good for first contribution Good for a first PR on the repo released on @feat-rewrite released scope: lib Anything related to the library itself state: has PR A PR is available for that issue type: feature This is a new feature

Comments

@maxime1992
Copy link
Collaborator

I spent 15mn debugging an AutomaticRootFormComponent trying to understand why the values where not passed from the parent down to the component on init (and never actually..).

Turns out it's Friday and I forgot to use DataInput 🤦‍♂️

I think it might be worth adding a runtime check to make sure we're using it on the *RootFormComponents.

@maxime1992 maxime1992 added scope: lib Anything related to the library itself effort-1: minutes Will only take a few minutes to fix/create state: needs design This feature or fix should be discussed before writing any code type: feature This is a new feature flag: good for first contribution Good for a first PR on the repo labels Feb 28, 2020
@ntziolis
Copy link
Contributor

ntziolis commented Mar 7, 2020

Checking for this surely helps, but I feel the best option is to add schematics to the package and ask people to please use those. This would also make sure one can never again forget to add the component providers (another one of those things that is easily forgotten).

@maxime1992
Copy link
Collaborator Author

Schematics would be nice but a check would definitely be required here IMO for people not using the schematics or using it on stackblitz etc

@maxime1992
Copy link
Collaborator Author

Discussed offline with @zakhenry and Ty:

  • Put a marker when the decorator is called
  • Simply throw an error when component instantiated if it doesn't have that marker

maxime1992 added a commit that referenced this issue Jun 15, 2020
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 maxime1992 added state: has PR A PR is available for that issue and removed state: needs design This feature or fix should be discussed before writing any code labels Jun 15, 2020
@maxime1992 maxime1992 self-assigned this Jun 15, 2020
@maxime1992
Copy link
Collaborator Author

🎉 This issue has been resolved in version 5.2.0-feat-rewrite.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@maxime1992 maxime1992 linked a pull request Oct 11, 2020 that will close this issue
zakhenry pushed a commit that referenced this issue Oct 23, 2020
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
Copy link
Collaborator Author

Just as an FYI @DataInput has been deprecated in the rewrite hence the message above

maxime1992 added a commit that referenced this issue Nov 21, 2021
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
@github-actions
Copy link

🎉 This issue has been resolved in version 5.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-1: minutes Will only take a few minutes to fix/create flag: good for first contribution Good for a first PR on the repo released on @feat-rewrite released scope: lib Anything related to the library itself state: has PR A PR is available for that issue type: feature This is a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants