karimayachi / karimayachi.github.io

MIT License
2 stars 0 forks source link

Abstracting away the view model #8

Open avickers opened 4 years ago

avickers commented 4 years ago

Looking at Alan's work in the future of Knockout thread, I think that you were right about abstracting away the view model with decorators.

I had hoped to avoid needing for people to write custom Babel configs for their bundler to get started (hence wanting to wait until stage 3), but since decorators are already supported by the TS compiler, I guess it would only apply to vanilla JS projects. That's probably good enogh.

karimayachi commented 4 years ago

I think the main goal is not to use decorators per se, but decoupling or loosely coupling the ViewModel from / to the library. I think I earlier mentioned something like this in the emails, but not on this repo, so here it is again on record 😄 :

I don't like how the VMs and library are coupled in Knockout through the use of observable functions:

let title = ko.observable('Hello World');

title('Hello Mars');

I don't like it for 3 reasons:

  1. It takes away from the reusability of code. Maybe this is a theoretical problem: in practice I've rarely encountered actual recycling of large amounts of business logic. Yet it's the thing that all frameworks/libraries always boast about.
  2. In large VMs it really has an impact on readability. Especially if for some reason you would need to have some properties non-observable and use them side by side with observable ones. This becomes confusing quick:
    title('Hello Venus');
    description = 'a new frontier';
  3. It's semantically incorrect. Properties/attributes are meant to get or set the state of part of an object. Functions/Methods are meant to execute functionality. Although I'm used to it after working with Knockout for many years, it still feels strange sometimes. Judging by the questions asked on the Knockout Google Group it also accounts for MANY bugs made by beginners that don't grasp it.

I think there are roughly two approaches to abstracting away the observables:

Automatically

Have the library convert any VM that is fed to it to have it's properties made observable. This should also be recursive and be done every time a new object (with properties that need converting) is assigned to another property or pushed onto an observable array.

Manually

Have the developer mark which properties should become observable. Decorators could be the cleanest method to achieve this, but it isn't the only one. Providing meta-data on the ViewModel could be another, just from the top op my head. I can probably think up some more methods.

Either way, automatically or manually, there are many ways to achieve this abstraction:

By the way, in Knockdown you use Proxies the other way around: you provide the VM and have the option to return a Proxy that reflects the properties to the original observable VM. That's also an interesting approach, but it would still leave the coupling during coding the VMs and the responsibility for making the observables with the programmer.

So, we would have to decide on whether we want automatic or manual conversion and on the method to achieve that. Decorators could come in handy for manual conversion. Decisions, decisions 😄

avickers commented 4 years ago

By the way, in Knockdown you use Proxies the other way around: you provide the VM and have the option to return a Proxy that reflects the properties to the original observable VM. That's also an interesting approach, but it would still have the coupling during coding the VMs and the responsibility for making the observables with the programmer.

Yes, this is to accommodate the use of method chaining during configuration. I'm not opposed to automatically wrapping and then proxying everything. I doubt anyone likes writing .observable() over and over again. If we go with automatic proxying, there needs to be a convenient way to unwrap and get at the underlying observable. A method like _unwrap() would cause issues for code that relies upon enumerating properties and wouldn't truly be decoupled, so I guess that leaves the Global Symbol registry? Implementation would need to be abstracted away.

import {Unwrap} from '@avickers/knockdown

Unwrap(vm.myObs).await(import(...))

// could also be aliased so that ko.unwrap() would work as well

Have the library convert any VM that is fed to it to have it's properties made observable. This should also be recursive and be done every time a new object (with properties that need converting) is assigned to another property or pushed onto an observable array.

Actually, this is what I was getting at when I asked if we should implement the functionality of ko.mapping: passing in an object and automatically making all of its properties observable. I must not have explained it clearly.

Although, automatically making all of the elements of an array observable was not something that I had considered. My initial reaction is, "Gee, what would the overhead look like for a length 50,000 array?" But, actually, I think it might be OK because I already have a plugin that handles those sorts of cases, where you will want pagination/virtual lists etc.

karimayachi commented 4 years ago

Actually, this is what I was getting at when I asked if we should implement the functionality of ko.mapping: passing in an object and automatically making all of its properties observable. I must not have explained it clearly.

Ah, sorry. I never use ko.mapping, so I may have missed your point there. ko.mapping creates a viewmodel with the same named properties as the data you fed into it. I don't think that is as clean as converting and replacing the original VM with a proxied version though...

Although, automatically making all of the elements of an array observable was not something that I had considered. My initial reaction is, "Gee, what would the overhead look like for a length 50,000 array?"

Maybe it's not a good idea. I don't know. We good just as well opt for the 'manual'-style. In which case I prefer the decorators-style of Alan's proof-of-concept. But it can be done without decorators.

karimayachi commented 4 years ago

Ps. I'm don't understand exactly what you mean with the 'unwrap'-problem.. Could you elaborate on that? Thanks!

avickers commented 4 years ago

Ps. I'm don't understand exactly what you mean with the 'unwrap'-problem.. Could you elaborate on that? Thanks!

I guess it's a question of what it does. Do we just automatically wrap everything in an Observable or ObsArray or do we go ahead and transparently proxy them too?

const data = {
  myObs: 5,
  myArray: [1,2,3,4]
}
const vm = ViewModel(data)

If we just wrap them then either you still call Knockdown.proxy(vm) (maybe ViewModel.proxy(vm) in this case) or properties reference the observable objects, i.e. vm.myObs.set(6)

Alternatively, ViewModel() could go ahead and return a Proxy, such that vm.myObs = 6 could be used. If we go with automatic proxying, then we are basically inverting the unwrap proposition from KO. In KO you unwrap the observable to get at the underlying values, in this scenario we would unwrap the Proxy and return underyling observable. Unwrap(vm.myObs).listenForKey('enter',handler) typeof vm.myObs === 'number' // true Unwrap(vm.myObs) instanceof Observable // true

I was just postulating a way to do this as unobtrusively as possible. I think Symbol would work nicely. That way, for instance, for(let p in vm.myObs) won't return _unwrap: function() ...

karimayachi commented 4 years ago

Ah, got it!

Yes, we would need to reach the underlying observable for subscribing, listenForKey in your example, etc... We could reference the observables from the proxied properties in a WeakMap. How would a Symbol based solution look?

I really enjoy cracking these little nuts! 😄

avickers commented 4 years ago

Well, let's see...

I'm thinking that ViewModel(vm, wrap?) we should allow a configuration param to determine whether to return a proxied or unproxied VM. If you know you're going to do a lot of work on the VM at the start, you can pass false and then manually proxy it after configuration.

As far as unwrapping a proxied VM goes, since my current implementation proxies at the VM level, the easier and more consistent solution is to unwrap at the VM level as well.

Modify the Proxy handler

const UNWRAP = Symbol.for('kd.unwrap')
let handler = {
      get: function(target, name, receiver) {
        if (name === UNWRAP && !target.hasOwnProperty(UNWRAP)) {
            return target
        }
        if(target[name] instanceof Observable) {
          return target[name].get()
        } else {
          return Reflect.get(target, name, receiver)
        }
      }
      set: ...
    }

The unwrap function

const Unwrap = function(vm,key) {
    return key
    ? vm[Symbol.for('kd.unwrap')][key]
    : vm[Symbol.for('kd.unwrap')]
}
const data = {
  test: "Hello World"
}
const vm = ViewModel(data)
console.log(vm.test) // Hello World
const view = Unwrap(vm)
console.log(view) // Object { test: Observable ... }
const test = Unwrap(vm,"test")
console.log(test) // Observable { _value: "Hello World" ... }

I guess the ViewModel class should probably have static methods unwrap() and proxy() that are aliases to Unwrap() and Proxy() to limit the number of explicit imports required.

karimayachi commented 4 years ago

Got it. Symbol property to have a non-enumerable way to retrieve additional data from the viewmodel. Yes, that's much cleaner than the __ko_* properties of KO.

I was thinking about storing references globally in a WeakMap. Somewhere along the lines of (from the top of my head.. will probably not be correct!):

// Or Unwrap... but unwrap would suggest that it returns something one way or the other, 
// while in this inverted case we specifically want the observabe, so return nothing if
// an observable is not found for this property
const getObservableFor = function(prop) {
    if(Knockdown.propertyMap.has(prop)) {
        return Knockdown.propertyMap.get(prop);
    }
}

The proxy handler:

let handler = {
      get: function(target, name, receiver) {
        if(target[name] && Knockdown.propertyMap.has(target[name])) {
          return Knockdown.propertyMap.get(target[name]).get(); // is that how you 'get' in KD?
        } 
        else if(target[name]) {
          return Reflect.get(target, name, receiver)
        }
      }
      set: ...
    }

Either by calling ViewModel(data) or automatically when binding, the WeakMap would by filled with observables that are referenced from the original properties. The observables are not 'stored' with the ViewModel.

Usage:

const data = {
  test: "Hello World"
}
const vm = ViewModel(data)
console.log(vm.test) // Hello World
const test = getObservableFor(vm.test)
console.log(test) // Object { test: Observable ... }

I think both methods will have pros and cons when it comes to garbage collecting and keeping references to observables that still have subscriptions, etc.. But I don't know what they are yet.

avickers commented 4 years ago

I think that it might be helpful to pivot back to the question of what it means to be a modern MVVM library, and address the issue of how we deal with global/shared state and the boundaries between VMs and between M/VMs. (Address the sort of issues that led to the Flux pattern and the SSoT.) This implementation will certainly end up touching upon that, and might help to inform it.

avickers commented 4 years ago

I think that I might have arrived at a satisfactory solution to this.

Knockdown instances will have the ViewModel attached via class getters and setters.

const ko = Knockdown(document)
const viewModel = { ... }

ko.vm(viewModel)

OK, so the solution is to create Observables, Proxies, and Dependents intelligently.

It's no longer a question of whether to observe and/or proxy everything automatically (wasting resources) or have to construct the view model imperatively.

If a binding is bound to a property on the viewModel, then that property is made observable and proxied.

If an expression inside of html`` depends on at least one value inside of the viewModel, then a Dependent is automatically created and any values depended upon are converted to Observables, if they aren't already. Such that you don't have to imperatively create Dependents (Computeds), you can simply use them declaratively.

const viewModel = {
  firstname: "Bert",
  lastname: "Bertington",
  nickname: "Berty"
}

class Home extends Koc {
 constructor() {
   super()
   const ko = this.ko()
   ko.vm(viewModel)

   this.html`
     <h1>Hello, ${this.vm.firstname +' '+ this.vm.lastname}</h1>
   `
   ko.applyBindings()  // inferred to be this.vm
 } 

 rename() {
    this.vm.firstname = "Bob" // the vm getter returns proxies
 }
}

The final state of this._vm, the backing variable, would look like this:

firstname instanceof Observable // true
lastname instanceof Observable // true
typeof nickname === "string" // true - still just a string because nothing subscribed to it.
_ko7 instanceof Dependent // true - declarative Dependents receive a UID
_ko7.get() === "Bert Bertington" // true

Meanwhile, you would be able to do this:

const home = new Home()
home.rename()
// home.vm._ko7 === "Bob Bertington"

Result

<h1>Hello, <span data-bind="html: _ko7">Bob Bertington</span></h1>"

Since Knockdown supports dynamic binding, if a binding is added later and it finds the property on the view model, it will only turn it into an Observable--or ObsArray--at that point in time. Meanwhile, if the property isn't located on the view model, the binding will remain inactive and Knockdown will keep track of it. If the property is dynamically added to the View Model later, it will be made observable and bindings applied.

Finally, it would still be possible to use my Observable methods one of two ways. One is that you could continue to imperatively define them when constructing the view model:

const viewModel = {
  widget: new Observable('<wl-spinner></wl-spinner>').await(import('/components/widget'))
}

Alternatively, you could access the backing variable directly prior to calling applyBindings()

this._vm.modal = ko.observable('<wl-button>Click me</wl-button').await(import('/components/modal_launcher'))