Hyperapp: Actions break when returning null

[FlorianWendelborn]

In app.js:77 hyperapp will crash if an action returns a value that is neither undefined nor typeof object (also for null). null.

Reason is that it tries to access result.then which is only possible for objects.

Also what should be the expected behavior if a function returns null? I used that in a previous version to get around return without any parameters. This lead me to discover this crash upon upgrading to v0.7.1.

E.g.

// abort if mode is `off`
if (mode.toLowerCase() === 'off') return
someFunctionCall()
// without semicolons this looks like it would return the next line
// similar to
const example = () =>
someFunctionCall()
// which would return the result of `someFunctionCall`
// just because you're aware that `return` terminates on newline doesn't mean that it doesn't look like it wouldn't/shouldn't

For now I'll use undefined, but it'd be good to have a defined behavior for null and other primitives.

Maybe we should introduce environment-specific errors like React does? They'd be amazing for development and can just get stripped away in production environments. There could be an error or warning in the case of null etc.

[jorgebucaran]

@dodekeract Good catch, it will crash if the return value is null. But it will not crash for other primitive types, since variable.nonExistentProp will just return undefined.

[jorgebucaran]

Also what should be the expected behavior if a function returns null? I used that in a previous version to get around return without any parameters. This lead me to discover this crash.

If your action is returning null, you are probably doing something wrong though.

[jorgebucaran]

without semicolons this looks like it would return the next line

Not sure I follow. It doesn't look like that to me.

Maybe we should introduce environment-specific errors like React does?

Yes, maybe later, I'd like to actually build some stuff first.

[FlorianWendelborn]

If your action is returning null, you are probably doing something wrong though. — @jbucaran

As long as I could handle effects as I wanted to this was not a problem. As I said previously I'm considering null to be an explicit undefined.

Also - is there a reason why returning null wouldn't just delete the model? null is explicitly meant to represent a "missing" object, so from a language point-of-view I'd expect it to delete the model or at least return like an effect.

Not sure I follow. It doesn't look like that to me. — @jbucaran

Updated the example.

[jorgebucaran]

@dodekeract I'd say don't use null.

Is there a reason why returning null wouldn't just delete the model?

That doesn't make sense.

[FlorianWendelborn]

Saying that it doesn't make sense doesn't make it true. I explained why it should be what's happening. If you have a counter-argument that's good. Otherwise what you said "doesn't make sense".

[jorgebucaran]

@dodekeract Deleting the model doesn't make sense because the model is immutable. Under the hood we mutate the model with a new model, but that's an implementation detail.

Perhaps what you meant to say was setting the model (or a property in the model) to its initial zero value?

An empty string for Strings, zero for numbers, [] for Arrays, and so on.

If that's the case, then you should return that value explicitly from within your actions.

[jorgebucaran]

@dodekeract We could support null easily by changing the line your mentioned to:

if (result == null || typeof result.then === "function") {
...

So, instead of ===, we can use == to coerce a variable which is null to pass the undefined check.

[FlorianWendelborn]

No. I meant what I said. There are cases where you just have "this model represents an arbitrary object" as a use-case. This would be a case where null would be desirable to present the absence of said object.

Alternatively just returning it would also be sane. My point is that there should be a defined behavior. You can't expect that everyone will remember that hyperapp doesn't like it when actions return null.

Also if a third-party-library returns null users would have to manually filter its return value for that.

[FlorianWendelborn]

@jbucaran That should work, yes. It'd lead to a defined behavior.

If the minifier supports it I'd suggest to use the more explicit result === null || result === undefined though, since it makes it obvious that the == is no mistake (it usually is).

[jorgebucaran]

SGTM

If you can send a PR with tests cool!

@dodekeract If the minifier supports it I'd suggest to use the more explicit...

Doesn't seem to do what we want. https://skalman.github.io/UglifyJS-online 😭

So, we'll use ~== undefined~ == null which should be a byte less :)

EDIT: Typo.

[jorgebucaran]

[leeoniya]

== null is what you want

[FlorianWendelborn]

@leeoniya Why is == null better than == undefined?

[leeoniya]

bytes

[FlorianWendelborn]

@leeoniya Weird. Any idea why the minifier doesn't support this? It turns undefined into void 0, but won't turn == undefined into == null.

[jorgebucaran]

@dodekeract Yes, because like you can see in the screenshot above, undefined is transformed into void 0 which is a wider than null.

[leeoniya]

plus you cannot redefine null, but you can redefine undefined in javascript (wtf, right?). if you want to explicitly test against undefined you have to use typeof x == "undefined". but == null should cover both cases since it will internally test against the built-in undefined as well as null. i use it everywhere internally in domvm, and it works well & is fast.