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

refactor(forms): Move FormControl to an overridden constructor. #44316

Closed
wants to merge 0 commits into from

Conversation

dylhunn
Copy link
Contributor

@dylhunn dylhunn commented Nov 30, 2021

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. In summary, this change causes the constructor for FormControl to be decoupled from its implementation. This will allow us to provide multiple constructor signatures in the future, and even to return FormControl<T> where T takes different values depending on the constructor signature selected. (For example, in the simple case, new FormControl<string>('foo') will return FormControl<string|null>.)

Accordingly, a new exported symbol is created to provide various constructor overrides, with documentation examples for how to use the different constructors. (Currently there are only two, but there will be more after Typed Forms/default values.)

Submitting this change separately is desirable for a few reasons:

  • It reduces the risk of landing Typed Forms -- the constructor override is complicated, and is fundamentally separate from the core types.
  • It enables us to submit a follow-up change for default values before Typed Forms lands, thus separating the only runtime behavior change from all the types-only changes.

This change should have no visible impact on users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

#13721

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

@google-cla google-cla bot added the cla: yes label Nov 30, 2021
@ngbot ngbot bot added this to the Backlog milestone Nov 30, 2021
@dylhunn dylhunn added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: presubmit The PR is in need of a google3 presubmit target: minor This PR is targeted for the next minor release and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Nov 30, 2021
@dylhunn
Copy link
Contributor Author

dylhunn commented Nov 30, 2021

Presubmit is clean (barring one flake). We'll need a TGP before submitting as well, just to be safe.

@dylhunn dylhunn marked this pull request as ready for review November 30, 2021 22:27
@sonukapoor
Copy link
Contributor

LGTM

packages/forms/src/model.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just left a couple comments 👍

packages/forms/src/model.ts Outdated Show resolved Hide resolved
packages/forms/test/form_control_spec.ts Outdated Show resolved Hide resolved
packages/forms/src/model.ts Outdated Show resolved Hide resolved
@mary-poppins
Copy link

You can preview 48883c0 at https://pr44316-48883c0.ngbuilds.io/.
You can preview 789c9aa at https://pr44316-789c9aa.ngbuilds.io/.

@dylhunn dylhunn force-pushed the fc-default-value branch 2 times, most recently from b17b2cb to d5015a1 Compare December 1, 2021 21:18
@dylhunn
Copy link
Contributor Author

dylhunn commented Dec 1, 2021

@AndrewKushnir I have made the requested changes.

@dylhunn dylhunn requested a review from AndrewKushnir December 1, 2021 21:25
@mary-poppins
Copy link

You can preview d5015a1 at https://pr44316-d5015a1.ngbuilds.io/.

@dylhunn dylhunn force-pushed the fc-default-value branch 2 times, most recently from a321277 to 953989c Compare December 2, 2021 04:23
@mary-poppins
Copy link

You can preview 953989c at https://pr44316-953989c.ngbuilds.io/.

dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 26, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 26, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 26, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 26, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 26, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 27, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 27, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 27, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 27, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 28, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Jan 31, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
jessicajaniuk pushed a commit that referenced this pull request Jan 31, 2022
…or. (#44316) (#44806)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close #44316

PR Close #44806
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 1, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 1, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 1, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 1, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 1, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 1, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 1, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 1, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 1, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 2, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 2, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 2, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 3, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 3, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
dylhunn added a commit to dylhunn/angular that referenced this pull request Feb 4, 2022
…or. (angular#44316)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 25, 2022
josmar-crwdstffng pushed a commit to josmar-crwdstffng/angular that referenced this pull request Apr 8, 2022
…or. (angular#44316) (angular#44806)

This implementation change was originally proposed as part of Typed Forms, and will have major consequences for that project as described in the design doc. Submitting it separately will greatly simplify the risk of landing Typed Forms. This change should have no visible impact on normal users of FormControl.

See the Typed Forms design doc here: https://docs.google.com/document/d/1cWuBE-oo5WLtwkLFxbNTiaVQGNk8ipgbekZcKBeyxxo.

PR Close angular#44316

PR Close angular#44806
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: forms cla: yes cross-cutting: types PullApprove: disable target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants