From 8a946aeaa1003b6a408305797c36151c63d1d7ec Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jacogr@gmail.com>
Date: Fri, 22 Feb 2019 10:50:48 +0100
Subject: [PATCH] Option create, order of checks (#733)

* Option create, order of checks

* Alphabetical
---
 packages/rpc-core/src/index.ts     | 12 ++++++------
 packages/types/src/codec/Option.ts | 23 +++++++++++------------
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/packages/rpc-core/src/index.ts b/packages/rpc-core/src/index.ts
index 90a561b049..f81730cc8b 100644
--- a/packages/rpc-core/src/index.ts
+++ b/packages/rpc-core/src/index.ts
@@ -195,7 +195,7 @@ export default class Rpc implements RpcInterface {
       const meta = key.meta || { default: undefined, modifier: { isOptional: true } };
 
       return meta.modifier.isOptional
-        ? new Option(Clazz, isNull(result) ? null : base)
+        ? new Option(Clazz, isNull(result) ? null : new Clazz(base))
         : new Clazz(base);
     } else if (method.type === 'StorageChangeSet') {
       // multiple return values (via state.storage subscription), decode the values
@@ -210,24 +210,24 @@ export default class Rpc implements RpcInterface {
 
         // see if we have a result value for this specific key
         const hexKey = key.toHex();
-        const item = (base as StorageChangeSet).changes.find((item) =>
+        const { value } = (base as StorageChangeSet).changes.find((item) =>
           item.key.toHex() === hexKey
-        );
+        ) || { value: null };
         const meta = key.meta || { default: undefined, modifier: { isOptional: true } };
 
         // if we don't have a value, do not fill in the entry, it will be up to the
         // caller to sort this out, either ignoring or having a cache for older values
         result.push(
-          !item
+          !value
             ? undefined
             : (
               meta.modifier.isOptional
                 // create option either with the existing value, or empty when
                 // there is no value returned
-                ? new Option(Clazz, item.value)
+                ? new Option(Clazz, value.isNone ? null : new Clazz(value.unwrap()))
                 // for `null` we fallback to the default value, or create an empty type,
                 // otherwise we return the actual value as retrieved
-                : new Clazz(item.value.unwrapOr(meta.default))
+                : new Clazz(value.unwrapOr(meta.default))
             )
         );
 
diff --git a/packages/types/src/codec/Option.ts b/packages/types/src/codec/Option.ts
index f89ddde785..9ec968907f 100644
--- a/packages/types/src/codec/Option.ts
+++ b/packages/types/src/codec/Option.ts
@@ -24,23 +24,22 @@ export default class Option<T extends Codec> extends Base<T> implements Codec {
   }
 
   static decodeOption<O> (Type: Constructor, value?: any): Codec {
-    if (isU8a(value)) {
+    if (isNull(value) || isUndefined(value) || value instanceof Null) {
+      return new Null();
+    } else if (value instanceof Option) {
+      return Option.decodeOption(Type, value.value);
+    } else if (value instanceof Type) {
+      // don't re-create, use as it (which also caters for derived types)
+      return value;
+    } else if (isU8a(value)) {
+      // the isU8a check happens last in the if-tree - since the wrapped value
+      // may be an instance of it, so Type and Option checks go in first
       return value[0] === 0
         ? new Null()
         : new Type(value.subarray(1));
-    } else if (value instanceof Option) {
-      return Option.decodeOption(Type, value.value);
     }
 
-    return isNull(value) || isUndefined(value) || value instanceof Null
-      ? new Null()
-      : (
-        // don't re-create when the type is correct - it could be derived from the base
-        // and at the same time since it exists, no need to have additional overhead
-        value instanceof Type
-          ? value
-          : new Type(value)
-      );
+    return new Type(value);
   }
 
   static with<O extends Codec> (Type: Constructor): Constructor<Option<O>> {