Improvements

[staeke]

Hey guys!

I thought I'd create the exact same thing as this repo. And...intended on naming it 'safe-proxy' too - only to find out you already created it :). So, I thought I'd ask you if you wanna allow me as a collaborator for joint work instead of me putting up a similar repo on npm. (I can start out with a PR)

Changes I have (in mind):

• Removing dependency on harmony polyfill (polyfilling should be external)
• using Symbol.toPrimitive instead of __value
• optional fallback value
• typescript support
• ISC license instead of MIT

[erictrinh]

Hi! Thanks for the suggestions, I do think it's safe to remove the polyfill now that the spec has finalized. To be honest, I hadn't been following the development of the Proxy spec closely. Now that it's been finalized and deployed to a bunch of environments, I'll brush up this repo 😄

Can you explain further how implementing Symbol.toPrimitive would replace __value? From my understanding, this would only work in a few cases, e.g.:

safe(thing).that.resolves.to.number.__value // the normal way to get the number inside the wrapped value
+safe(thing).that.resolves.to.number // gives you a number instead of a wrapped value
'' + safe(thing).that.resolves.to.string // gives you a string instead of a wrapped value

As far as I know, there doesn't seem to be a way of eliminating the unwrapping of the value you need at the end, at least not universally. What did you have in mind?

[staeke]

Hi!

I guess I was a little vague. First off, my implementation returned the actual values for primitive values, rather than the safe proxy, so it was only "safe" as long as you went to objects/null/undefined. Which I think is ok for 99.9% of the cases. But...not strictly necessary. So, what are the cases where you want to "unwrap" your safe proxy, i.e. access its underlying value, and can it be done automatically?

• [Symbol.isPrimitive] - automatic, string concats/arithmetics etc. (probably very common, not least for template generation)
• toJSON - automatic, also very common (passing to various API:s, logging etc.)
• console.log representations - automatic on node.js via util.inspect.custom, not necessary in browsers, and cousins
• typeof ... - depending on approach, will always return object with your approach. With mine, it would still be correct
• instanceof ... - returns correct

Other cases, such as when you pass an argument to another API will sooner or later become one of the categories above (unless I'm missing some important case). Hence, __value seems to be of little value.

Also, the name __value seems a little arbitrary now that we have symbols. I think some exported symbol (maybe safe.value) would be appropriate. This is, provided that it's not very often you'd use it.

[erictrinh]

Your mention of console.log representations made me realize that console.log is completely broken at the moment. :smile: When node tries to console.log a safed object, it blows the call stack. Thanks for the heads up, I'm working on fixing this.

So the reason I'm wary of automatic unwrapping is it breaks the contract of "safe." If I have a nested object and I am calling deep into it, I may unexpectedly hit a primitive value, keep chaining, and get an error. For instance,

const config = {
  moreNestedConfig: true // woops, this is supposed to be an object
};

safe(config).moreNestedConfig.oh.no // TypeError
//                           ^ unwrap happens, you're on your own

Personally, I would find that behavior unexpected. Plus, I sort of like the explicit unwrapping!

I agree that __value is not an ideal name. As you guessed, it's there to avoid collisions with other property names you might have on your safed object. I do like your idea of using an exported symbol, and I actually don't think it looks bad! If you already have safe imported, it's not too cumbersome to do [safe.value] instead of .__value.

safe(config).nested.obj.__value // before
safe(config).nested.obj[safe.value] // after

The only issue is it doesn't read as cleanly, since you are doing property access with brackets instead of dots, but that may be an acceptable compromise.

[staeke]

Cool, so let's break things apart here. It feels like we're talking about some different things

Should safe automatically unwrap for primitives You provided a good example, which was what I meant with my 0.01% of the cases. And let me add, I agree with you. It might be an esoteric use case, but it's much easier to understand and reason about an API that is "always safe", rather than one that is safe unless you go to keys below an undefined property of an existing primitive. And I believe easy = good.

However, there's also a tradeoff here. By having automatic unwrapping for primitives you can get typeof to work without unwrapping. And I do think that's important too.

Maybe provide two versions? safe and safe.unwrapPrimitives? Or maybe it's overkill.

Should safe provide good handling of Symbol.toPrimitive, toJSON, and util.inspect.custom (in node) I think this is a no-brainer - yes

What should be the name of the unwrap symbol? I agree with you. My vote goes for [safe.value]. Saw you already implemented this. Awesome!

Typescript definitions I like typescript, and with features that are on their way to be rolled out (mapped types, mapped recursive types), it's nice to provide them. I'm happy to make a PR

In order to be clear, here is a code reference:

function isPrimitive(obj) {
    switch(typeof obj) {
        case 'boolean':
        case 'string':
        case 'number':
        case 'symbol':
            return true;
    }
    return false;
}

function safe(obj) {
  const isPrim = isPrimitive(obj);
  const inner = isPrim || !obj ? {} : obj;
  return new Proxy(inner, {
      get: function (target, key) {

          // Primitive
          if (key === Symbol.toPrimitive) {
              return function(hint) {
                  if (isPrim || obj == null)
                      return obj;
                  if (Symbol.toPrimitive in obj) {
                      return obj[Symbol.toPrimitive](hint);
                  }
                  return obj.toString();
              }
          }

          // JSON
          if (key === "toJSON") {
            return 'toJSON' in obj ? obj.toJSON : function() { return obj };
          }

          const res = target[key];

          // Automatically unwrap, run this line
          // return isPrimitive(res) ? res : safe(res);

          // Otherwise, this
          return safe(res);
      }
  });
}