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.

Sign up for GitHub

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

Jump to bottom

Can't use a <form> tag with a [formControl] directive #38391

Closed
kara opened this issue Aug 9, 2020 · 3 comments
Closed

Can't use a <form> tag with a [formControl] directive #38391

kara opened this issue Aug 9, 2020 · 3 comments
Assignees
@AndrewKushnir
Labels
area: forms forms: directives P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: confirmed state: has PR type: bug/fix
Milestone
Backlog

Comments

@kara
Copy link
Contributor

kara commented Aug 9, 2020 •
edited

🐞 bug report

Affected Package

The issue is caused by package angular/forms.

Is this a regression?

I don't think this is new behavior

Description

It's not currently possible to wrap a [formControl] directive in a <form> tag because an error is thrown by NgControlStatusGroup that the ControlContainer is missing. This isn't right because [formControl] directives are intended to be used without a ControlContainer.

The problem seems to be caused by NgControlStatusGroup having too broad of a selector, so that it activates on all form tags.

Workarounds:

  • You can circumvent the problem by adding ngNoForm to the parent <form> tag, but that is not intuitive
  • You can also work around the problem by adding the FormsModule so NgForm is automatically created. However, this isn't really correct because it requires mixing the forms modules.

🔬 Minimal Reproduction

https://stackblitz.com/edit/angular-ivy-cke9q5?file=src%2Fapp%2Fapp.component.html

If you load that Stackblitz, you can see the error in the console.

🔥 Exception or Error


preview-e378c6d150742202f755b.js:1 ERROR Error: NodeInjector: NOT_FOUND [ControlContainer]
    at getOrCreateInjectable (core.umd.js:4047)
    at Object.ɵɵdirectiveInject (core.umd.js:14362)
    at NodeInjectorFactory.NgControlStatusGroup_Factory [as factory] (forms.umd.js:1058)
    at getNodeInjectable (core.umd.js:4141)
    at instantiateAllDirectives (core.umd.js:8464)
    at createDirectivesInstances (core.umd.js:7831)
    at Object.ɵɵelementStart (core.umd.js:14511)
    at AppComponent_Template (app.component.ts:18)
    at executeTemplate (core.umd.js:7804)
    at renderView (core.umd.js:7613)

🌍 Your Environment

Angular Version:
Ivy, v10
@ngbot ngbot bot modified the milestone: needsTriage Aug 9, 2020
@mlc-mlapis
Copy link
Contributor

@kara Nice to hear from you again. 🥰

You can circumvent the problem by adding ngNoForm to the parent <form> tag, but that is not intuitive.

Yep, it's a bit hidden feature, but still, even with this directive, the console prints NOT_FOUND [ControlContainer].

@AndrewKushnir AndrewKushnir added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Oct 23, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Oct 23, 2020
AndrewKushnir added a commit to AndrewKushnir/angular that referenced this issue Jan 7, 2021
@AndrewKushnir
fix(forms): handle standalone <form> tag correctly in `NgControlSta…
8d0ecf5 
…tusGroup` directive

The `NgControlStatusGroup` directive is shared between template-driven and reactive form modules. In cases when
only reactive forms module is present, the `NgControlStatusGroup` directive is still activated on all `<form>`
elements, but if there is no other reactive directive applied (such as `formGroup`), corresponding `ControlContainer`
itoken is missing, thus causing exceptions (since `NgControlStatusGroup` directive relies on it to determine the
status). This commit updates the logic to handle the case when no `ControlContainer` is present (effectively making
directive logic a noop in this case).

Alternative approach (more risky) worth considering in the future is to split the `NgControlStatusGroup` into
2 directives with different set of selectors and include them into template-driven and reactive modules separately.
The downside is that these directives might be activated simultaneously on the same element (e.g. `<form>`),
effectively doing the work twice.

Resolves angular#38391.
AndrewKushnir added a commit to AndrewKushnir/angular that referenced this issue Jan 7, 2021
@AndrewKushnir
fix(forms): handle standalone <form> tag correctly in `NgControlSta…
d2714c4 
…tusGroup` directive

The `NgControlStatusGroup` directive is shared between template-driven and reactive form modules. In cases when
only reactive forms module is present, the `NgControlStatusGroup` directive is still activated on all `<form>`
elements, but if there is no other reactive directive applied (such as `formGroup`), corresponding `ControlContainer`
token is missing, thus causing exceptions (since `NgControlStatusGroup` directive relies on it to determine the
status). This commit updates the logic to handle the case when no `ControlContainer` is present (effectively making
directive logic a noop in this case).

Alternative approach (more risky) worth considering in the future is to split the `NgControlStatusGroup` into
2 directives with different set of selectors and include them into template-driven and reactive modules separately.
The downside is that these directives might be activated simultaneously on the same element (e.g. `<form>`),
effectively doing the work twice.

Resolves angular#38391.
@AndrewKushnir AndrewKushnir mentioned this issue Jan 7, 2021
Closed
3 tasks
@AndrewKushnir AndrewKushnir self-assigned this Jan 7, 2021
@atscott atscott closed this as completed in fdbd3ca Jan 8, 2021
atscott pushed a commit that referenced this issue Jan 8, 2021
@AndrewKushnir @atscott
fix(forms): handle standalone <form> tag correctly in `NgControlSta…
b3f322f 
…tusGroup` directive (#40344)

The `NgControlStatusGroup` directive is shared between template-driven and reactive form modules. In cases when
only reactive forms module is present, the `NgControlStatusGroup` directive is still activated on all `<form>`
elements, but if there is no other reactive directive applied (such as `formGroup`), corresponding `ControlContainer`
token is missing, thus causing exceptions (since `NgControlStatusGroup` directive relies on it to determine the
status). This commit updates the logic to handle the case when no `ControlContainer` is present (effectively making
directive logic a noop in this case).

Alternative approach (more risky) worth considering in the future is to split the `NgControlStatusGroup` into
2 directives with different set of selectors and include them into template-driven and reactive modules separately.
The downside is that these directives might be activated simultaneously on the same element (e.g. `<form>`),
effectively doing the work twice.

Resolves #38391.

PR Close #40344
@AndrewKushnir
Copy link
Contributor

@kara thanks for reporting the issue! The fix was merged and it's now available in google3 :)

@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 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@AndrewKushnir AndrewKushnir

Labels
area: forms forms: directives P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: confirmed state: has PR type: bug/fix
Projects
None yet
Milestone
Backlog
4 participants
@sonukapoor @kara @mlc-mlapis @AndrewKushnir