Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibly null values in the input data? #20

Open
esamattis opened this issue Sep 18, 2017 · 6 comments
Open

Possibly null values in the input data? #20

esamattis opened this issue Sep 18, 2017 · 6 comments

Comments

@esamattis
Copy link

I'm using strictNullChecks in my project. Updating an object with missing values will result in a runtime exception like TypeError: Cannot read property 'bar' of undefined.

Here's an example. Am I doing something wrong here?

import * as monolite from "monolite";

interface IState {
    foo?: {
        bar?: {
            baz?: number;
        };
    };
}

const state: IState = {};

const newState = monolite.set(state, s => {
    const foo = s.foo;
    if (foo) {
        const bar = foo.bar;
        if (bar) {
            return bar.baz;
        }
    }
    return undefined;
})(345);

console.log(newState);
@kube
Copy link
Owner

kube commented Sep 18, 2017

I will check that after work. Some work needs to be done on Monolite to be adapted to last TypeScript features.

@kube
Copy link
Owner

kube commented Sep 18, 2017

Proxy

The problem comes from the fact that the accessor function takes as argument a Proxy created by Monolite, which is returned recursively to take all accessors from root.

This means that if (foo) and if (bar) will always be truthy, as they are this same proxy injected as the s argument.

This is a limitation of Monolite. It was just designed to enable static typing on the classic array of accessors used in Lodash or Immutable.

Static Optimization

This function is not meant to be really executed at runtime (it can but won't be compatible with older browsers, and will be less performing), and needs to be optimized statically. (See https://github.com/kube/babel-plugin-monolite)
By optimizing it statically we can convert _ => _.foo.bar to ['foo', 'bar'].

So here I would recommend to do _ => _.foo!.bar!. At least for now, as I understand it's not elegant nor safe, but what we want is to tell where we want to go in the object.

Optional Subproperty

Where we should stop in the object is something that should be determined by the library itself, maybe once we hit the first subproperty that is undefined, and then do nothing. (and return the same object without modification)

This behaviour needs to be defined.

I'm open to propositions.

@kube
Copy link
Owner

kube commented Sep 18, 2017

I see that you also did like the last comment in microsoft/TypeScript#16 (comment). ;)

This Optional Chaining Operator is something I'm waiting for too. Monolite maybe needs to be reworked to be more compatible with upcoming features of JS and TypeScript.

@esamattis
Copy link
Author

esamattis commented Sep 18, 2017

Ah, makes sense now that I understand how this library actually works :)

The babel-plugin and the non-null-assertion operator probably is the most reasonable approach for now, but here's an idea:

Why not use a recursive proxy and force it's type for the path getting function?
Here's an example how it could be implemented: https://codesandbox.io/s/znojlkwox4
The typing information is what desired and there is no chance of runtime exceptions. Just try it on the codesandbox.io site - it has type aware autocomplete. The babel plugin could be then used optionally for performance and old browser support.

EDIT: Misunderstood again. monolite already uses this approach. The exception I was seeing was coming from the setting phase not the path getting.


But there is indeed a problem how the missing sub properties should be handled. I think ideally monolite should just create them like lodash.set does but how would mononite know how to create them? The itermediate objects might be Objects or Arrays.

I don't think doing nothing would be a good idea because it will cause errors go unnoticed. It would be better to return a maybe type (T | null). Return null when the update failed. Or just throw.


Yeah, the optional chaining operator is easily the most anticiated TS/JS feature for me.

@esamattis
Copy link
Author

Maybe just create objects if the intermediate objects are missing and require usage of nested monolite calls when an array update is required? Like https://codesandbox.io/s/507z7665kl

@kube
Copy link
Owner

kube commented Sep 23, 2017

I just saw this new library https://github.com/facebookincubator/idx.
It uses the same trick of accessor function, though they don't seem to use the same runtime Proxy-based resolution.
It makes me think that Monolite could also expose a get function to do Optional Chaining (on objects only) once something is decided for about optional subproperties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants