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

option to autocast strings as numbers #11464

Closed
rizen opened this issue Mar 30, 2015 · 14 comments
Closed

option to autocast strings as numbers #11464

rizen opened this issue Mar 30, 2015 · 14 comments

Comments

@rizen
Copy link

rizen commented Mar 30, 2015

There needs to be a way to cast strings as numbers from the model. Angular is used in all sorts of systems where you don't have access to the backend that generates the JSON that populates a field. So if I set a field like this:

<input type="number" ng-model="some_value">

Then angular should trust me when I say that some_value is a number, and run Number() on it.

I know you guys have considered this before, as you did in this ticket: #6683

And I know you've rejected the premise. However, that's just a terrible way to do things. We need a way to at least optionally change a config setting to change this behavior. If you don't, I'm forced to either set the field as "text", which is semantically wrong, or I'm force to loop over all the results as they come in and manually run Number() on them, which adds a lot of code for something that should be automatic. Please reconsider.

@Narretz
Copy link
Contributor

Narretz commented Mar 30, 2015

I'll think it's worth reconsidering this. Here's some thoughts:

  • the reason for this was that you will overwrite your non-number / string with a number once the user changes the input. You may not want this. Likewise, on init the model and view are inconsistent - one is a string , one is a number (which might be problematic for validation)
  • You can use a custom formatter that runs before the built-in formatter and formats your string to a number. No looping required.

@rizen
Copy link
Author

rizen commented Mar 30, 2015

That's why I said to make it an option. It may not be right to turn this on for everybody, but it's certainly good for people that explicitly want that behavior.

Could you link to an example of how to build a custom formatter? I've never heard of such a thing. Unless you mean a custom filter? If that's what you mean I can't apply the filter to the field because I've attached the model directly to the input field, so I can't do {{some_value | number}}. Please show me the error of my ways.

@rizen
Copy link
Author

rizen commented Mar 31, 2015

After a few hours of searching and reading, I finally figured out what an angular formatter is and how to apply it to this situation. For people seeing this going forward, I applied @Narretz advice and came up with this:

https://gist.github.com/rizen/5f76357f6758dd8f6f74

@Narretz
Copy link
Contributor

Narretz commented Sep 21, 2015

Hey @rizen if you want to give a PR a shot that allows non-Number numbers, that'd be great. Here's a list of things that'd be needed:

  1. add an option to ngModelOptions: numberAutocast or similar
  2. change the input[number] formatter to read this option and allow non-numbers in that case
  3. Update the docs.

@adamreisnz
Copy link

adamreisnz commented Jul 4, 2017

I am also in favour of this, due to the following use case:

I am calculating some numbers that I need to put in the model. However, these numbers are currency values, and as such I would like to have them formatted as such, e.g. with 2 decimal spaces, even if the value is 12.5, it should display as 12.50 in the input field.

A user can input 12.50 in the input[number] field without problems, so I think we should be able to put that kind of value into the input field through the model value as well.

Currently, this is not possible, because if I run the value through the number filter for example, it converts into a string and then Angular throws the ngModel:numfmt error.

So I have no way of pre-populating the number field with valid numbers at this stage, other than changing the input field to a text type, which of course is not ideal.

Ideally, input[number] would in fact accept strings when they come from the model and display them as they are, if they are a valid number.

Edit: I might actually create a new issue for this, as this issue is more than 2 years old now.

@frederikprijck
Copy link
Contributor

frederikprijck commented Jul 4, 2017

@Narretz If this is still something you'd like to have in the core, I'd be happy to create a PR for it. After looking into #16080 it looks like we might benefit from adding it in the core.

@gkalpak
Copy link
Member

gkalpak commented Jul 4, 2017

As already discussed, a formatter is the way to go. Formatters essentially provide a way for developers to implement their own "switches" to turn features on, either on specific inputs (e.g. with a custom selector that you have to explicitly apply to the inputs) or for all input of a type (e.g. using input as the selector and checking for the type).

Finally, as a very last resort, it is even possible to remove or replace the built-in formatters.

Additionally, in latest versions it is even possible (even if not well documented) to get hold of the ngModelOptions options and implement this as an ng-model-options property (if one wants to).
(There would still need to be a custom directive or decorating one of the existing ones.)

Personally, I would close this as won't fix, because what you want it already possible (without any changes to core), can be applied granularly and there is no intention (afaict) to change the default behavior atm (as this would be a breaking change of something that is useful in most cases).

I'll leave the final call to @Narretz 😃

@adamreisnz
Copy link

adamreisnz commented Jul 4, 2017

I don't think I agree. See the discussion in #16080 and this Plunkr: https://plnkr.co/edit/8XhP9xLhgNgx6RdAz9ZJ?p=preview

The input[type=number] actually accepts valid numerical String values natively as demonstrated by the Plunkr. E.g. something like this:

document.getElementById('input1').value = '2.50';

This sets the proper value for that input to 2.50, trailing zero included.
Note that it won't accept invalid string inputs, like 'INVALID' or even '12.75INVALID'.

So Angular's behaviour actually limits us in this case, preventing us from fully utilizing the native capabilities of the input[type=number]. That's why I think this is a problem with the framework, and not something to be solved in userland with custom formatters/decorators/hacks.

The simplest way to fix this in Angular core would be to mimic the native behaviour, and instead of just type checking if a Number is passed, it could check a String to be of a valid numeric format and put that in as is without casting it to a Number. This could be implemented with an optional additional flag, if you're worried about breaking changes, although personally I don't see it breaking many things.

Do you happen to know if Angular 2 has the same behaviour? Because I would argue that if this is something worth fixing in Angular 2, then Angular 1 should eventually be brought in line as well, as is being done with many other aspects.

@adamreisnz
Copy link

Looking at the source code, there already is a regex in place to check for valid numbers:

https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L28

All that would realistically need to change imo, is these lines:

https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L1519-L1521

Replace the isNumber() check in the formatter with the same regex check as is used a few lines above in the parser, and change the error message.

@adamreisnz
Copy link

adamreisnz commented Jul 4, 2017

I have tested this in our own project by creating a custom directive and it seems to work as expected;

/**
 * Module definition and dependencies
 */
angular.module('Shared.Input.Directive', [])

/**
 * Directive
 */
.directive('input', function() {
  return {
    require: [
      '?ngModel',
    ],
    link(scope, element, attrs, ngModel) {

      //Array?
      if (angular.isArray(ngModel)) {
        ngModel = ngModel[0];
      }

      //Number regex
      const NUMBER_REGEXP = /^\s*(-|\+)?(\d+|(\d*(\.\d*)))([eE][+-]?\d+)?\s*$/;

      /**
       * Helper to apply number handling
       */
      function applyNumberHandling() {

        //Remove existing formatter
        ngModel.$formatters.pop();

        //Roll our own
        ngModel.$formatters.push(function(value) {
          if (!ngModel.$isEmpty(value)) {
            if (!NUMBER_REGEXP.test(value)) {
              throw Error('Expected `' + value + '` to be a number');
            }
            value = value.toString();
          }
          return value;
        });
      }

      //Apply better number validation
      if (ngModel && attrs.type === 'number') {
        applyNumberHandling();
      }
    },
  };
});

This basically removes the existing formatter and replaces it with the one I proposed in the previous post, using Angular's own internal NUMBER_REGEXP for consistency.

@frederikprijck
Copy link
Contributor

frederikprijck commented Jul 4, 2017

@adamreisnz I had a chat offline with @gkalpak and it makes perfectly (or atleast some) sense as, in AngularJS, we have a seperation of ViewValue and ModelValue. When working with an API that expects a number, we want to ensure the modelValue is a number type (not a numeric value) hence there's no need to pass in a string. We could argue that we could pass in '2.50' and parse it to a number before doing numerical operations, but we'll again lose the trailing 0.

If you have the need to visualize the value differently from the model (visualize it as a string, while the model is a number) you can make use of an simple formatter like so: http://plnkr.co/edit/NJSUppF28ZPKsrWisXZF?p=preview

Even tho I understand your point (and I find it odd to have different behavior compared to the way native JS handles it aswell), I also understand @gkalpak's point of view with regards to api usage.
I also think it's going to be more complex than only allowing strings (depending on the numeric API usage internally).

@adamreisnz
Copy link

adamreisnz commented Jul 4, 2017

Ok, thanks for looking into it and for posting your suggestion, it's also a good approach.

Not sure if it would be more complex, as the regex is already there, and all I did was basically replace the isNumber check with the regex check. You use the same check in the parser, so it should be ok.

@gkalpak
Copy link
Member

gkalpak commented Jul 4, 2017

It seems we have come to some sort of consensus 😃
Still, here is a (not so) quick summary of my thoughts:

The question is not whether we can implement this (we obviously can 😃), but whether this is the right thing to do (imo it is not 😃).

What the browser does (i.e. accepting any string that is a valid representation of a number as value) is not really relevant, because AngularJS enhances the browser's capabilities and does better than the browser 😛

In AngularJS, there is a clear separation between the model (actual data) and the view (the visual representation of that data). NgModelController provides parsers and formatters (validators are not relevant here btw) for the exact purpose of giving the developer full control over how the data is presented to the user and how the user's input is translated to the model representation.

Using formatters for their intended purpose (i.e. formatting the model data for visual representation in the view) is not a hack or work around. It is normal and common practice (in fact AngularJS itself uses these APIs under the hood for all supported input types).

Now, specifically regarding the number input type, there is a contract in AngularJS that the model data bound to such an input is a number (which makes perfect sense imo, since the input is supposed to offer an interface for interacting with numbers). This guarantee allows AngularJS, 3rd-party libraries and app developers to operate on a common assumption that will always hold true: The model value (if not empty) will be of type number. (The same is true for other types of inputs, such as date etc.)

Changing this, would leave us in a position were we can't be sure of the type of the model data. This would affect several ngModel APIs (e.g. parsers, validators, formatters) - both in AngularJS itself and 3rd-party libraries - as well as any app-specific logic (methods, filters etc) that takes such data as input (and naturally expects to operate on numbers).

Furthermore, allowing this, would create another inconsistency which makes ngModel-bound inputs harder to reason about: You can't be sure what model data a specific input value corresponds to (or even what is the type of model data).
(BTW, saying that we will immediately update the model data to be a number would break another ngModel contract: As long as the input's validity hasn't changed, there can be no change in the model value, unless the user initiated that change (or the model value has been programmatically updated)).

In a nutshell, by implementing the requested change (and hiding it behind a flag in order to avoid a breaking change) we would be breaking some contracts that make ngModel easier to reason about and introducing undeterministic behavior.

These are obvious downsides. What would we gain in exchange?

AFAICT, the only tangible upside is being able to display a value in a specific format. But (as discussed earlier) this is exactly what formatters are for. In addition, using formatters has the following advantages:

  • They are more flexible (every app can customize them for its specific usecase).
  • They are composable (you can add other formatters in the pipe in arbitrary locations).
  • They can be applied selectively (on specific inputs) or app-wide (all number inputs). Or you can even apply them app-wide, but have certain inputs opt-out.

Even if the ngModel API is far from perfect, a considerable amount of work has been put into making it powerful and extensible. This is a great thing that should be utilized more imo. It allows developers to have their highly customized inputs, but without bloating the framework with code and logic that is not useful to all users.

And given how easy it is to re-use logic across AngularJS apps, I really see no reason for not solving this via formatters 😁

@adamreisnz
Copy link

Ok let's close this issue then 👍

@Narretz Narretz closed this as completed Jun 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants