angular / angular

Deliver web apps with confidence 🚀
https://angular.dev
MIT License
95.29k stars 25.04k forks source link

Forms should throw error if two or more NgModels appear in form with the same name #13993

Open kemsky opened 7 years ago

kemsky commented 7 years ago

[ ] bug report => search github for a similar issue or PR before submitting [x] feature request [ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

Currently there is no error or warning if two form controls have the same name:

<input name="entireStateCoverage" type="checkbox" [(ngModel)]="item.entireStateCoverage">
<input name="entireStateCoverage" type="checkbox" [(ngModel)]="item.active">

Expected behavior

Forms should throw error if duplicate names are detected.

What is the motivation / use case for changing the behavior?

This leads to unpredictable results (value overwrites) which are difficult to find.

angular-robot[bot] commented 3 years ago

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

angular-robot[bot] commented 3 years ago

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.

dylhunn commented 2 years ago

We'd likely accept a contribution in this area, and it's good for a first-time contributor. The message would go in forms/src/directives/template_driven_errors.ts, and the check would be somewhere in forms/src/directives.

phalgunv commented 2 years ago

I'd like to work on this issue.

phalgunv commented 2 years ago

How do I build Angular source code? When I run npm install I get the below error: `PS C:\code\angular> npm install npm ERR! code ERESOLVE npm ERR! ERESOLVE unable to resolve dependency tree npm ERR! npm ERR! While resolving: angular-srcs@14.1.0-next.0 npm ERR! Found: typescript@4.7.2 npm ERR! node_modules/typescript npm ERR! typescript@"~4.7.2" from the root project npm ERR! npm ERR! Could not resolve dependency: npm ERR! peer typescript@">=4.4.2 <4.7" from @angular/compiler-cli@13.3.11 npm ERR! node_modules/@angular/compiler-cli npm ERR! peer @angular/compiler-cli@"^13.0.0 || ^13.3.0-rc.0" from @angular-devkit/build-angular@13.3.6 npm ERR! node_modules/@angular-devkit/build-angular npm ERR! npm ERR! Fix the upstream dependency conflict, or retry npm ERR! this command with --force, or --legacy-peer-deps npm ERR!

npm ERR! A complete log of this run can be found in: npm ERR! C:\Users\pvaddepa\AppData\Local\npm-cache_logs\2022-06-08T19_35_31_088Z-debug.log`

Even npm install --force errors out. I am using node 16.10.0

Please let me know what I'm missing.

hansireit commented 2 years ago

@phalgunv The angular project uses yarn instead of npm. So just install yarn with "sudo npm install --global yarn" and then install the packages with yarn "yarn install". I would also like to contribute to this project, do you want to work on this issue together?

phalgunv commented 2 years ago

@phalgunv The angular project uses yarn instead of npm. So just install yarn with "sudo npm install --global yarn" and then install the packages with yarn "yarn install". I would also like to contribute to this project, do you want to work on this issue together?

Sure. Let's team up. My twitter handle is @PhalgunV

GrizzlyEnglish commented 1 year ago

@phalgunv Are you still working this? If not I'd like to give it a shot. Thanks!

dylhunn commented 1 year ago

@GrizzlyEnglish @phalgunv If one of you ends up implementing this, here is a code example of how to throw an Angular RuntimeError: Link to example.

As in the example, you should add a new RuntimeError enum value for the new error type.

dylhunn commented 1 year ago

Also @phalgunv I'm sorry I didn't see your request for setup help above. Long story short, it's much easier to get the build working on MacOS or Linux. I'd encourage you to dual-boot Linux (or possibly use WSL) to build Angular itself. I only have a Mac, so it's tough for me to offer Windows debugging advice. It sounds like @hansireit might be able to help you on Windows though

GrizzlyEnglish commented 1 year ago

@dylhunn Thanks! I have it all done, except one use case -> ngFor renaming elements being marked as dupes. Once I get that figured out should be good, unless @phalgunv still wants to work it; I don't mean to snatch it.

phalgunv commented 1 year ago

Thanks @dylhunn. I was stuck with for quite a long time due to build issues on Windows I have now switched to Ubuntu(via WSL).

@GrizzlyEnglish, I was probably only halfway through the changes and may not be able to get completed before you. So it seems fair if you'd continue with the fix.

dylhunn commented 1 year ago

Glad to hear you're working on this! No rush, but please send it my way and/or ping me when you want a review!

GrizzlyEnglish commented 1 year ago

@dylhunn Not sure if directed at me; but, I do have a PR open in draft (there was a bit of review done) and I seem to not have access to re-open the PR. Can you peek it and reopen please?

dylhunn commented 1 year ago

@GrizzlyEnglish I have reopened #46994. Is it ready for review? Edit: yes; convo continued on the PR

dylhunn commented 1 year ago

Unmarking as help wanted, since we have an almost-done PR that's likely to be finished soonish

vp4140 commented 10 months ago

Hey @dylhunn , I'm interested in continuing working on the above mentioned PR.