No selections setting view model to undefined rather than null

[talikan]

• I'm submitting a ...
  [ ] bug report
  [ ] feature request
  [x ] question about the decisions made in the repository
  [ ] question about how to use this project

• Summary
  It would appear that de-selecting all option choices ends up setting the external model binding to undefined rather than null. This seems innocuous on the face of it, but has caused us a problem that leads me to believe this should be changed.

In our project we evaluate objects sent back to our services for changes between the existing values in our database and new values sent back from the client. w11k-select sets properties to undefined when all options are unselected; when we send our object back to the server, Angular strips all undefined properties off before serializing the object out to JSON. Since our services don't see a newly "null" value for the property, since it wasn't sent, the change isn't recorded.

While I can certainly fix this for our project, this seems like it might be a common use case; in addition, properties in Javascript simply shouldn't be set to undefined... explicitly saying "the value has been unset" is more appropriately a use case for null. If that's so, then w11k-select should be modified to correctly set the NG Model to null when no options are selected.

• Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. StackOverflow, personal fork, etc.)

[talikan]

Assuming the maintainers agree, I'm willing to write a patch to this effect.

[pburgmer]

Hi @talikan,

Thank you very much for your detailed description and your offer to implement it.

I agree setting the property to null seems to be the better way. But as a lot of users already use w11k-select and rely on the way it works now, I don't feel so comfortable to change this behaviour.

w11k-select is a pure AngularJS directive. I think most people starting a new project will do it using Angular instead of AngularJS. So the change you suggest would mostly affect existing projects. I see a huge bug potential caused by missing type checking. Changing undefined to null can break a lot of existing code.

Therefor I suggest not to change it. What do you think about it? And @CanKattwinkel what's your opinion?

[CanKattwinkel]

I agree with you about the breaking changes. Particularly with regard to the age of the projects and the underlying framework, I would not make any changes here either.

[talikan]

@CanKattwinkel and @pburgmer: I both understand and agree with the concern for existing projects. This could be a breaking change if any are relying on the undefined value.

Despite the age of the project, however, I'd argue that there are many thousands of AngularJS projects out there that are still being actively developed (including mine), and they may come to use w11k-select at or after this point (as mine did).

I propose a compromise. I've created a branch in my fork (https://github.com/w11k/w11k-select/compare/develop...talikan:provide-nullable-model-config-option?expand=1) that provides a new config option, 'useNullableModel', and fixes the issue. This config option is defaulted to false, preserving the existing undefined output of the project for existing usage, but permitting new usage, including my own, to use a nullable model.

If you find this an acceptable path, I'm happy to create the PR for it. If you truly believe it shouldn't be incorporated, I'll step back and continue to use my fork for our project.

Thanks for taking a look!

[pburgmer]

@talikan

Thanks for your feedback. Your compromise with the additional configuration sounds very good to me. Could you please create a pull request. @CanKattwinkel could you then please review the pull request.

[CanKattwinkel]

Resolved by dfdb007 in version 0.11.3.

Thanks for your pull request @talikan

[talikan]

Thanks for the dialogue!