Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(error/areq): describe your change... #14980

Closed
wants to merge 2 commits into from

Conversation

nikhilknoldus
Copy link

@nikhilknoldus nikhilknoldus commented Aug 3, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

docs Document Update.

What is the current behavior? (You can also link to an open issue here)

Application stops, when this error occur we get official link but this does not solve the error.

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

Other information:

Encountered this error many times, the provided details are very encrypted (don't seem practical), i have added a line that is 99% practical and main cause for this error. These 3 points that i have added are seriously core reasons for this error.

Encountered this error many times, the provided details are very encrypted (don't seem practical), i have added a line that is 99% practical and main cause for this error. These 3 points that i have added are seriously core reasons for this error.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

1 similar comment
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@nikhilknoldus
Copy link
Author

I signed it.

@googlebot
Copy link

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Aug 3, 2016
@nikhilknoldus
Copy link
Author

nikhilknoldus commented Aug 3, 2016

@Narretz @vitvad CI failed due to your server timeout, not mine. Please review.

@Narretz
Copy link
Contributor

Narretz commented Aug 3, 2016

The error is too specific. areq is not only thrown for controllers, but for other parts of the code, too.

@nikhilknoldus
Copy link
Author

@Narretz your view is right but the doc's lines are not worth at all, written so complicatedly, number of angular developers i know who open this bug line from browser console, read it and say what's this? They don't infer anything from this detail. The comment which i have written matching major error criteria and the earlier doesn't.

@Narretz
Copy link
Contributor

Narretz commented Aug 3, 2016

But your change only mentions controller, which is is also confusing. If someone gets the error without having ngController, then this is not helpful at all.
I'm okay with including some note about syntax errors, but it should be less specific. If you have example code that shows a common example how this error can happen, then we can include it.

@Narretz Narretz modified the milestones: Ice Box, Purgatory Aug 3, 2016
Earlier commit was focusing on controller only, but current commit is covering large area of this error, that is mostly the reason.
@nikhilknoldus
Copy link
Author

nikhilknoldus commented Aug 4, 2016

@Narretz Martin your view is again right in this case, but as i mentioned, syntax error and earlier error doesn't infer it, and yes my commit is focusing on controller as you said, i can modify it for larger criteria. And for the example thing: my project is quite big (for an international bank), i do have lots of controllers too and in that case watching this error horribly blank our(js developers) mind when something like this happens: Examples:
1- angular.module('someApp').controller(-'SomeCntrl', SomeCntrl);
*notice the hyphen before. it may be any symbol, char etc. That happens by mistake obviously.

and this is only 1 scenario there are much that we face alot.

I have updated my commit as well.

Thanks !!

@nikhilknoldus
Copy link
Author

Hi @Narretz any update regarding same.

Narretz added a commit to Narretz/angular.js that referenced this pull request Aug 11, 2016
…tered

Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller did
not yield a function. Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes angular#14980
@Narretz
Copy link
Contributor

Narretz commented Aug 11, 2016

I've taken another look and I think you are right that this error is confusing when it is thrown with for controllers. I've opened a PR that throws a better error here: #15015

@nikhilknoldus
Copy link
Author

Hey thanks Martin for considering my PR.

Narretz added a commit that referenced this pull request Aug 12, 2016
Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller is not defined. 
Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes #14980
PR (#15015)
@nikhilknoldus nikhilknoldus deleted the patch-1 branch August 13, 2016 05:54
Narretz added a commit that referenced this pull request Aug 17, 2016
Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller is not defined. 
Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes #14980
PR (#15015)
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller is not defined. 
Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes angular#14980
PR (angular#15015)
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller is not defined.
Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes angular#14980
PR (angular#15015)
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller is not defined.
Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes angular#14980
PR (angular#15015)
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller is not defined. 
Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes angular#14980
PR (angular#15015)
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller is not defined.
Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes angular#14980
PR (angular#15015)
petebacondarwin pushed a commit that referenced this pull request Nov 23, 2016
Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller is not defined.
Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes #14980
PR (#15015)
petebacondarwin pushed a commit that referenced this pull request Nov 24, 2016
Previously, it would throw the ng:areq error, which is less
specific and just informs that the requested controller is not defined.
Given how commonly controllers are used
in Angular, it makes sense to have a specific error.

The ng:areq error is still thrown when the registered controller
is not a function.

Closes #14980
PR (#15015)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants