Mossaka / client-node

3 stars 1 forks source link

Handle default parameters in node client #3

Open Mossaka opened 3 years ago

Mossaka commented 3 years ago

Current implementation does not handle default parameters such as cf.

If we do

client.get("k4") 

nodejs will return a unhelpful error message:

UnhandledPromiseRejectionWarning: TypeError: failed to downcast any to string
    at internal/util.js:297:30
    at new Promise (<anonymous>)
    at Object.<anonymous> (internal/util.js:296:12)
    at RawClient.get (/home/mossaka/developer/LFX/client-node/index.js:20:26)
    at /home/mossaka/developer/LFX/client-node/app.js:18:18
(node:24468) UnhandledPromiseRejectionWarning:

See more about Js default parameters: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Default_parameters

See more about how Neon handles default/optional arguments: https://neon-bindings.com/docs/arguments

Mossaka commented 3 years ago

I have an experiment implementation of the default parameters in snapshot::scan

let start = cx.argument_opt(0).map(|start| {
            start
                .downcast::<JsString, _>(&mut cx)
                .or_throw(&mut cx)
                .unwrap()
                .value(&mut cx)
                .into_bytes()
        });
let end = cx.argument_opt(1).map(|end| {
            end.downcast::<JsString, _>(&mut cx)
                .or_throw(&mut cx)
                .unwrap()
                .value(&mut cx)
                .into_bytes()
        });

This same method can be applied to other methods. There is a potential opportunity to extract the common logic out to utils.rs

Mossaka commented 3 years ago

It turns out that the above implementation handles the optional arguments when the length of the arguments is smaller than the required ones. A correct solution should expand on the handling of downcasting.

Mossaka commented 3 years ago

Another task:

cf in RawClient should be completely optional and we should provide a default value to it.