search

strongloop / strong-remoting

Communicate between objects in servers, mobile apps, and other servers.
www.strongloop.com
Other
105 stars 93 forks source link

Required string argument should reject empty string #263

Closed STRML closed 8 years ago

STRML commented 8 years ago

In practice, you're going to want to be guarding this anyway, so strong-remoting may as well do it.

Given all the sloppy http coercion, there's in practice little difference between undefined, null, and ''. If I mark a string argument as required, I expect there to be an actual string there. Thoughts?

If you agree, (and also merge #260), I can form https://github.com/STRML/strong-remoting/commit/8a68689428ba8d097b631063d84a2461b8763922 into a PR.

STRML commented 8 years ago

Additionally, if we do go this route, I think it makes sense to fail if an array argument is empty in combination with required.

richardpringle commented 8 years ago

@bajtos (in case you haven't noticed, I'm doing a little spring cleaning on the strong-remoting backlog) what do you think about this?

I tend to agree with @STRML but I'm not sure about our current users. This might be a good candidate for our next release since there is already a PR.

@STRML, sorry about the lack of attention, we're improving but still have a long way to go.

bajtos commented 8 years ago

+1 for rejecting empty string for required string args. I am not sure about rejecting empty arrays for required array args, perhaps we can add an option to control this behaviour?

I am taking a deeper look at the coercion issues in strong-remoting, let's fix this as part of that work.

bajtos commented 8 years ago

This should be fixed by https://github.com/strongloop/strong-remoting/pull/343 in the latest 3.0-alpha.