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

Unhelpful error message when Component is missing a selector #3464

Closed
jelbourn opened this issue Aug 4, 2015 · 12 comments
Closed

Unhelpful error message when Component is missing a selector #3464

jelbourn opened this issue Aug 4, 2015 · 12 comments
Labels
effort1: hours help wanted An issue that is suitable for a community contributor (based on its complexity/scope). hotlist: error messages type: bug/fix

Comments

@jelbourn
Copy link
Member

jelbourn commented Aug 4, 2015

If you omit selector completely for a Component, you get

EXCEPTION: The selector "undefined" did not match any elements

If you use an empty string as a selector, you get an error in view_manager.js

EXCEPTION: TypeError: Cannot read property 'componentDirective' of undefined

Both of these cases should be captured by a single check that tells the user something like

Component "SourdoughBreadcrumbs" does not have a selector.
@btford
Copy link
Contributor

btford commented Aug 5, 2015

Related to #2336.

@mhevery mhevery added comp: core effort1: hours help wanted An issue that is suitable for a community contributor (based on its complexity/scope). type: bug/fix labels Aug 6, 2015
@danrasmuson
Copy link
Contributor

Question: How should we reference "SourdoughBreadcrumbs" in the error message? I imagine we would normally use the component selector however in this example it doesn't exist, right?

A generic error message might look like...

annotations_impl/annotations.ts

!isString(selector) ? throw makeTypeError('The component annotation requires a string selector'): null;

Give me your thoughts and I'll create a pull request and make some tests.

@pkozlowski-opensource
Copy link
Member

@danielrasmuson we had some conversation about it in #3533 and it looks like TypeScript tape checking should take care of this for people using TS.

@mhevery I guess we still need a check inside ng2 for people using ES6 / ES5, right? If so I would be more happy to help out @danielrasmuson to prepare a PR

@mhevery
Copy link
Contributor

mhevery commented Aug 17, 2015

Sure @danielrasmuson can send a PR.

@danrasmuson
Copy link
Contributor

@pkozlowski-opensource I've started working on this but I have a few questions.

  • Selectors that are not a string are invalid, correct?
  • Selectors that are an empty string are invalid, correct?
  • Should we validate the selector for special characters, '~!&*...'?
  • Is not providing a selector valid? (See Feat: components without a selector #1662)

@pkozlowski-opensource
Copy link
Member

@danielrasmuson empty, invalid or omitted selectors should throw. I would start simply and verify just those, we can add more checks (eg. special chars) later on.

danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Aug 20, 2015
…component annotation

if the component selector is an empty string, not a string, or not supplied an error will be thrown

Closes angular#3464
@0x-r4bbit
Copy link
Contributor

Doesn't that conflict with: #1662 ?

@pkozlowski-opensource
Copy link
Member

@PascalPrecht @danielrasmuson yeh, thinking about it some more, I think that throwing in ViewMetadata is probably too early. We should only throw only when a selector of a given directive is being used.

The other case I can think of where an empty selector in ViewMetada might make sense is when we would like to have some conventions (ex. selector derived from a class name)

danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Aug 26, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Aug 26, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Sep 1, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Sep 1, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Sep 1, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Sep 1, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Sep 1, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Sep 10, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
@danrasmuson
Copy link
Contributor

@pkozlowski-opensource I've updated my pr with your feedback. It's only a couple lines. Your thoughts? #3759

danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Sep 29, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
@SonicGD
Copy link

SonicGD commented Oct 16, 2015

There is some problem with selector validation:

Failed to execute 'querySelector' on 'Document': 'div[foo=123]' is not a valid selector.

It fails only if attribute value is int. String value is fine:

div[foo=bar] 

Check on alpha44

vicb pushed a commit to vicb/angular that referenced this issue Oct 27, 2015
…component annotation

If the component selector is an empty string or not a string an error
will be thrown.

Closes angular#3464
Closes angular#3759
danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Oct 28, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
danrasmuson pushed a commit to danrasmuson/angular that referenced this issue Oct 28, 2015
…component annotation

If the component selector is an empty string or not a string an error will be thrown.

Closes angular#3464
@vicb
Copy link
Contributor

vicb commented Oct 28, 2015

#3759 was trying to catch the error in the selector. However as discussed with @tbosch the check should be in the compiler.

tbosch added a commit to tbosch/angular that referenced this issue Jul 27, 2016
Components without a selector now get the selector `ng-component`.
Directives without a selector will throw an error message.

Closes angular#3464
Closes angular#10216
tbosch added a commit to tbosch/angular that referenced this issue Jul 28, 2016
Components without a selector now get the selector `ng-component`.
Directives without a selector will throw an error message.

Closes angular#3464
Closes angular#10216
tbosch added a commit to tbosch/angular that referenced this issue Jul 28, 2016
Components without a selector now get the selector `ng-component`.
Directives without a selector will throw an error message.

Closes angular#3464
Closes angular#10216
@hansl hansl closed this as completed in 9b39e49 Jul 28, 2016
@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 Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
effort1: hours help wanted An issue that is suitable for a community contributor (based on its complexity/scope). hotlist: error messages type: bug/fix
Projects
None yet
9 participants