farmOS / field-kit

A modular, offline-first companion app to farmOS.
https://farmOS.org
GNU General Public License v3.0
62 stars 39 forks source link

Improving `farmLog` with Symbols and Metaprogramming #358

Closed jgaehring closed 4 years ago

jgaehring commented 4 years ago

My foray into JavaScript Symbols last week led me to an interesting article by Keith Cirkel called "Metaprogramming in ES6: Symbols and why they're awesome". The intro to Metaprogramming is pretty fascinating in its own right, but of particular interest to me and farmOS is the section on using Symbols for metadata.

I'm thinking about this as a possible way to improve the set of farmLog functions. For instance, we could add some additional functions for accessing metadata:

function farmLog = (logTypes) => {
  const changed = Symbol('changed');
  const conflicts = Symbol('conflicts');
  const data = Symbol('data');

  return {
    createLog() {
      // ...
    },
    // ...
    getLastChange(log, key) {
      return log[key][changed];
    },
    getConflicts(log, key) {
      return log[key][conflicts];
    },
  };
}

This also got me thinking more about using dedicated getters and setters which accessed the symbolic properties instead of directly referencing the string property. I played around with this in the console a bit:

let str = 'name'; // undefined
sym = Symbol(str); // Symbol(name)
let log = {
    [sym]: {
        changed: Date.now(),
        conflicts: [],
        data: 'foo',
    },
    get [str]() {
        return log[sym].data;
    },
} // undefined
log.name // "foo"

This could be refined with Object.defineProperty() method:

// Inside the `createLog` method
const now = Math.floor(Date.now() / 1000);
const log = {};

// Inside each iteration of the schema entries
const key = 'name'; // this would actually be a function parameter, but for illustrative purposes
const sym = Symbol(key);

Object.defineProperty(log, sym, {
  enumerable: false,
  value: {
    [changed]: now,
    [conflicts]: [],
    [data]: props[key],
  }
})

Object.defineProperty(log, key, {
  enumerable: true,
  get() {
    return this[sym][data];
  },
  set(val) {
    this[sym][changed] = Math.floor(Date.now() / 1000);
    this[sym][data] = val;
  }
})

I'm eager to play around with this more. I think this will be really helpful when it comes to exposing an API for Field Modules to access the metadata on logs, for instance, if they want to display conflicts generated while syncing so the user can reconcile them. It may also make it possible to really simplify the API for updating logs, since with the getter/setter pattern it's now just a matter of assignment, log.name = 'Harvest kale', and the changed timestamp will be updated automatically. It will also make it easier to work with the original objects, since the metadata is non-enumerable, so iterating over the properties will not include that metadata.

jgaehring commented 4 years ago

This could be a problem for using Symbols in Vuex (from the Vuex docs):

The data you store in Vuex follows the same rules as the data in a Vue instance, ie the state object must be plain. See also: Vue#data

jgaehring commented 4 years ago

This could be a problem for using Symbols in Vuex

Ehh, maybe not:

https://codesandbox.io/s/symbol-logs-5d3xk

Seems to work in that example, although the getters and setters are pretty useless since they're being overwritten by the Vue observer. I don't know why getOwnPropertySymbols() is returning [ null ] though, while getLastChange still seems to work. I've opened a question on the Vue forum. Hopefully I get some kind of response, because I'm stumped.

jgaehring commented 4 years ago

Another issue with Symbols would be retaining metadata when caching logs in IndexedDB.

jgaehring commented 4 years ago

Seems to work in that example, although the getters and setters are pretty useless since they're being overwritten by the Vue observer. I don't know why getOwnPropertySymbols() is returning [ null ] though, while getLastChange still seems to work.

Alright, some lessons learned after diving into the Vue source, particularly its defineReactive function: https://github.com/vuejs/vue/blob/6fe07ebf5ab3fea1860c59fe7cdd2ec1b760f9b0/src/core/observer/index.js#L132-L194

I was mostly being thrown off by how the Symbols weren't being handled by JSON.stringify. It seems that the Symbol properties were being propagated through the Vuex store all along, as I think my updates to the Codesandbox show. Crucially, however, they are not being overwritten by the reactiveGetter and reactiveSetter in defineReactive, so they are not reactive. I found a workaround by setting the name property on the log then passing the log back into my addLog mutation, but it feels pretty hacky.

I'm going to keep exploring this, but I think the getter/setter pattern won't be very useful within Vue. Probably in other contexts, but for my use case we probably want to keep using something like the updateLog function, which returns a new object each time.

jgaehring commented 4 years ago

This is also interesting: https://forum.vuejs.org/t/class-property-not-reactive-when-setter/6852/3?u=jgaehring

Maybe reactivity is still possible with your own getter/setter?

jgaehring commented 4 years ago

The secret sauce:

        configurable: true,

Totally works now! :rocket:

but I think the getter/setter pattern won't be very useful within Vue

No, it's going to be so useful! Particularly with Vuex mutations. Totally makes sense that it just had to be made configurable. Woohoo!

jgaehring commented 4 years ago

Another great metaprogramming talk focused on ES6 Proxies:

https://www.youtube.com/watch?v=_5X2aB_mNp4

The accompanying repo:

https://github.com/eiriklv/json-populate

This means of creating JSON-like graphs by linking $ref properties could be a really useful way of linking between different entities in FK.

jgaehring commented 4 years ago

I'll probably do a little more testing with this, and will certainly follow up with #401, but I think this new implementation is good to go!