Angular.js: Introduce a way for a component to require an attribute

Created on 24 Jul 2017  路  12Comments  路  Source: angular/angular.js

I'm submitting a ...

  • [ ] bug report
  • [X] feature request
  • [ ] other (Please do not submit support requests here (see above))

Problem description:

Let's have a component (for simpleButton) with following bindings:

bindings: {
    name: '@',
    enabled: '<',
    onClick: '&',
  },

And it needs to be used with all the bindings set like this <simple-button enabled="true" name="Foo" onClick="foo()"></simple-button>. A programmer shouldn't be able to use it without one or more non-optional attribute like this <simple-button></simple-button> or <simple-button enabled="true" name="Foo"></simple-button>.

There's no way to ensure that all required bindings will be set. This causes unpredictable behavior and it's a bit hard to debug if any of them is forgotten.

https://plnkr.co/edit/OwrnxuU3QhsvyNBMSrXh?p=preview

Current behavior:

There's no way to enforce that a binding in the component is required and must be set as an attribute.

If a binding is non-optional (without ?) and no value is given it's set to undefined.

At the moment <simple-button enabled="undefined"></simple-button> and <simple-button></simple-button> are indistinguishable from each other (when non-optional).

Expected / new behavior:

It throws an error if required binding is not set as an attribute of a component.

Angular version:

latest

Anything else:

I see two possible ways to solve this.
The first is to enforce all bindings without ? to be set as an attribute of a component. But I can see that may be a bit too radical. I tried and it worked but broke 105 tests :/
The second is to introduce a new marker that will indicate that attribute must be set. ! looks like a good candidate.

I'm willing to submit a PR if there's a decision how to proceed :)

$compile feature

Most helpful comment

There's now a config option that makes directives throw when non optional bindings are missing. https://github.com/angular/angular.js/commit/f1d01bbc748e033035107dbb4259fe40d3443dfb

All 12 comments

For what it's worth: Angular doesn't have this option neither (unless I'm missing something). Even tho I like the idea, I'm not sure if it's a good idea to introduce something that's going to make upgrading only harder.

I also remember this has been discussed before but I can't seem to remember which issue/pr that was or what the outcome was. Maybe @gkalpak or @Narretz can shed their light ?

If it helps, Vue seems to have a concept of required props, which seems close https://vuejs.org/v2/guide/components.html#Prop-Validation .

In Angular (2+), it would be easy to create a @Required decorator using elementRef.nativeElement - since you decorate each attribute, it could just look for that name in nativeElement's props.

In AngularJS (1) I see no nice way of achieving that... (apart from accessing angular internals from the controller to read the parsed bindings and comparing that to $attrs)

Technically, props in AngularJS that aren't marked as optional are required, but an absent attribute doesn't trigger a warning or an error. The reason why this hasn't been added is that it would possibly break many apps, because not every directive author has used optional bindings even though they should. So a change at this point could be quite disruptive for questionable gain. We could introduce error throwing for missing required attributes behind a flag though,.

See also here https://github.com/angular/angular.js/pull/6064

An updated PR that addressed https://github.com/angular/angular.js/pull/6064#issuecomment-64928346 could be considered, whether directly or behind a flag.

@Narretz thanks.. I understand this has historical reasons, but.. if we introduced a new modifier to mean strictly required, that wouldn't break anybody, right?

(At least, I assume that ! is not a valid part of an attribute name, so =! would not clash with = +
!weird_name_startign_with_an_exclamation_mark)

@Narretz could it be done through new $compileProvider option or through same .debugInfoEnabled(), that already exists? I would assume, that in production it shouldn't break anything, since in production .debugInfoEnabled() shouldn't be set to true. And if it brakes in development environment do not see any issue with that. Would additional bullet in Breaking changes section in CHANGELOG.md if it even would be considered as breaking change.

@jeserkin I don't see this strictly as debug info. Debug Info data should not be relied upon in production, but required attributes is something you should be able to rely upon. A new compileProvider option sounds better.

@himdel I'd rather not introduce a new modifier for a behavior that is technically already there. ! would be "strict requirement" and I think this is better handled by introducing a flag on the compileProvider or making throwing the default - which would benefit all directives, not only those that use !. The Directive Defintion object is complicated enough.

Understood...the debugInfo idea would work for us, even just seeing a warning when that happens, doesn't necessarily have to be an exception.

All we really need is to clue the developers that they have forgotten something :). (Or that the component got updated since and is misssing something now.)

@Narretz does something like $compileProvider.strictComponentBindingsEnabled(true/false) sound eligible for implementation and PR?

@jeserkin Sounds good to me :)

@ZitaNemeckova as I understood, you gonna do a PR to add this feature, correct? Or you've only created and issue?

@jeserkin I will make a PR :) Thank you for finding nice way to solve our issue :)

There's now a config option that makes directives throw when non optional bindings are missing. https://github.com/angular/angular.js/commit/f1d01bbc748e033035107dbb4259fe40d3443dfb

Was this page helpful?
0 / 5 - 0 ratings

Related issues

WesleyKapow picture WesleyKapow  路  3Comments

jpsimons picture jpsimons  路  3Comments

brijesh1ec picture brijesh1ec  路  3Comments

kishanmundha picture kishanmundha  路  3Comments

jtorbicki picture jtorbicki  路  3Comments