calmm-js / partial.lenses

Partial lenses is a comprehensive, high-performance optics library for JavaScript
MIT License
915 stars 36 forks source link

Optic doesn't support Symbol. #166

Open jituanlin opened 6 years ago

jituanlin commented 6 years ago

Well, Optic is support String, Array ...etc However, the ES6 new type Symbol is mostly support by modern browser and Node.js. But, if we create a new Optic like this:

[
  Symbol(),
  'string'
]

The error is throw.

polytypic commented 6 years ago

Yes, currently Partial Lenses do not support symbols and in non-production mode the library raises an exception as seen here.

I'm not opposed to supporting symbols, but I'm not entirely sure what the best way to support them would be. Ramda, for example, does not currently support symbols. It seems that the use case of symbols is to avoid name clashes. Even JavaScript itself seems rather undecided on how symbol properties should be treated: object spread ignores them, while Object.assign copies them as seen here. Object spread and Object.assign copy symbols as seen here.

I did some quick tests and it seems that copying symbol properties (using builtin Object.assign or Object.getOwnPropertySymbols) would cause something like a 2x performance hit in latest Node for property lenses. Note that I mean this in the sense of supporting the possibility of having symbol properties—I actually tested this on objects that did not have symbol properties, so there was no extra cost of actually copying more. Hopefully future JS engines will have faster implementations of Object.assign and then supporting symbol properties could be done without taking a performance hit.

For the above two reasons, namely

I'd rather not enhance the default property lenses in Partial Lenses to support symbol properties at this point. The above concerns may change in the future (IOW, a consensus may be reached and copying symbol properties might get significantly faster) at which point this could be reconsidered.

Note that you can always write custom optics for accessing objects with symbol properties. Here is a simple example using L.lens and here is a version that also provides the symbol or string as the index (Oops: Fixed!). At the moment this is what I'd recommend if you need to access objects with symbol properties.

Also, it might be useful to add the above sym lens to the library. Possibly with a better name? Name suggestions and/or PR is welcome!

jituanlin commented 6 years ago

I read your example. However, the variable name like xi2yF confuse me. What it's name mean? When I want to read this project source code. This problem tangle me, I want to contribute my code, but first , how to understand this, which knowledge I miss. Thanks

polytypic commented 6 years ago

Ah... Good question. xi2yF spells out the signature of the parameter: it is a function that maps the value under scrutiny x and its index i to the resulting value y in functor F.

polytypic commented 6 years ago

Also, my latter example had a bug—I just fixed the link.

jituanlin commented 6 years ago

Got it ,thanks

Justin-ZS commented 6 years ago

@polytypic Hi, I think the object spread demo should be {...x, y: 1} rather than {...foo, y: 1}. In this case, the behavior is same to Object.assign

polytypic commented 6 years ago

@Justin-ZS Oops. Thanks for the correction! Here is the fixed playground.

ftaiolivista commented 3 years ago

If someone, like me, needs to not loose Symbols using lenses this a patch I have done to infestines.


diff --git a/node_modules/infestines/dist/infestines.cjs.js b/node_modules/infestines/dist/infestines.cjs.js
index 3d711b4..a1a4f30 100644
--- a/node_modules/infestines/dist/infestines.cjs.js
+++ b/node_modules/infestines/dist/infestines.cjs.js
@@ -438,14 +438,8 @@ var assocPartialU = function assocPartial(k, v, o) {
   var r = {};
   if (o instanceof Object) {
     if (!isObject(o)) o = toObject(o);
-    for (var l in o) {
-      if (l !== k) {
-        r[l] = o[l];
-      } else {
-        r[k] = v;
-        k = void 0;
-      }
-    }
+    // Symbol Support
+    Object.assign(r, o);
   }
   if (isDefined(k)) r[k] = v;
   return r;
@@ -456,10 +450,9 @@ var dissocPartialU = function dissocPartial(k, o) {
   if (o instanceof Object) {
     if (!isObject(o)) o = toObject(o);
     for (var l in o) {
-      if (l !== k) {
-        if (!r) r = {};
-        r[l] = o[l];
-      } else {
+      // Symbol Support
+      if (!r) {r = {...o}}
+      if (l === k) {
         k = void 0;
       }
     }