Aurelia Store, immerjs and objects

1

Still experimenting things with Aurelia Store and immerjs. I have a component which receive an article object so it can display it. This article comes from the store. On the component, there is a button to mark the article as read. When it is clicked, an action is dispatched to mark the article as read (API request + change in the store). When I try to update the article in the store with:

export const markedAsRead = produce((draft: IState, article: IArticle) => {
    // If we access article directly, immerjs has no knowledge of the modification and the state is mutated.
    const index = articles.map((article) => article.id).indexOf(articleToFind.id);
    draft.rss.unreadArticles[index].isRead = true;
});

I get this error: Unhandled rejection Error: Immer drafts cannot have computed properties. If I test with an object that is not passed to a view model, it works fine (for instance by putting the code below in the constructor of the App class):

 const originalArticle = new Article({status: null});
const baseState = {articles: [originalArticle]};
const nextState = produce(baseState, (draft) => {
    const index = draft.articles.map((article) => article.id).indexOf(originalArticle.id);
    const article = draft.articles[index];
    article.status = 'Read';
});
console.log(nextState.articles[0]);
console.log(originalArticle);

From what I was able to track down, it comes from the fact that Aurelia defined getters and setters on objects for each properties. Because of that, immerjs canā€™t clone the objects and thus canā€™t work. From what I read here, it actually makes sense from the immerjs perspective:

The problem with getters is that they canā€™t be cloned. What should be cloned? The value? But in that case they become ā€˜staticā€™. Or should the descriptor be copied? But in that case the closure of the getter might be wrong, that is, referring to something it was already referring to, there is no way to tellā€¦

Any thoughts on this? I guess I can always revert to the spread operator but it feels like defeating the point of immerjs.

1 Like
2

I am not sure if it is the right way, but I used an older version of immer (1.8.2) which worked with this setup. In the end I gave up on using Immer and just used JSON.parse(JSON.stringify()) or Object.assign().

The Aurelia State examples did not work for me with the newest version of Immer.

R

1 Like
3

It sounds like a nice addition to ease state management. And we have an example for it so I find it a bit disappointing that it is not working.

Maybe we have a way to ā€œun-aureliaā€ the object to remove these getters and setters and make immer work. @zewa666 do you have an opinion on this?

1 Like
4

Frankly I havenā€™t worked with Immer.js anymore aside from the time Iā€™ve created the sample. So you say the sample as it is now with the latest update breaks? Iā€™ll have a look what we can do instead.

1 Like
5

The example may work because you are only using an int. The problem seems to arise only when you pass an object to a view model like: my-value.bind="myObject"because Aurelia will add setters and getters to myObject.

2 Likes
6

I gave it another look but this is above what I can figure out on my own. I managed to make two sandboxes to illustrate the issue. If I bind an int like in the example, everything works: https://codesandbox.io/s/aurelia-sandbox-t8nch If I bind an object (here a simple/standard object), I get my error: https://codesandbox.io/s/aurelia-sandbox-gh3so (I actually have to go to https://gh3so.codesandbox.io/) to view the error in the console.

1 Like
7

one cheap quickfix i used was @connectTo(store => store.state.pipe(pluck('myObject'), map(cloneDeep)))

1 Like
8

This could work yes.

Where does cloneDeep come from?

1 Like
9

lodash for example. could also x=>JSON.parse(JSON.stringify(x)) i believe

3 Likes
10

@doktordirk thanks for the precision. I thought about something like that. That does work. Thanks.

I also wanted to try something a bit different: create a custom bindable decorator to avoid to explicitly clone the data. I tried but itā€™s above my skills in Aurelia right now ^^. Based on https://github.com/aurelia-contrib/aurelia-typed-observable-plugin/blob/master/src/patches.ts#L58 (which does something similar to add validation), I managed to get to this, which looks quite ugly to me:

const originalSetValue = BehaviorPropertyObserver.prototype.setValue;

BehaviorPropertyObserver.prototype.setValue = function(this: BehaviorPropertyObserver, newValue: any) {
    let copiedValue = newValue;
    if (typeof newValue === 'object') {
        copiedValue = cloneDeep(newValue);
        // copiedValue = JSON.parse(JSON.stringify(newValue));
    }
    originalSetValue.bind(this)(copiedValue);
};

But I always get this error: TypeError: this.getArrayObserver(...) is undefinedaurelia-binding.js:4814

Anyway, I donā€™t think Iā€™ll spend more time on this. I think itā€™s a bit sad that this doesnā€™t work more easily out of the box. I also find the deep cloning a bit inefficient (mostly given how immer works to avoid data copying). I really liked immerā€™s approach to the problem which I find very clear and readable.

@zewa666 were you able to find another options?

1 Like
11

@Jenselme Iā€™m fighting this exact battle right now. Using Object.assign in a complex app with deeply nested states is cumbersome. I really thought immer.js was the way to go until I hit this issue. Iā€™ve been googling for the last 3 hours and no one seems to have a handle on this other than magic strings which I detest:

const newObj = immutable(obj).set(ā€˜a.bā€™, ā€˜fā€™)

Really, thatā€™s how the javascript kiddies code in the age of Typescript and IDEā€™s?

There are a few type safe libraries that have like 8 stars and havenā€™t been updated in a year so Iā€™m staying away from that.

Watching this thread closely so @zewa666 can come up with something brilliant.

1 Like
12

@zewa666 pinged me to take a look at this.

So as you folks may or may not know, there is a spectrum of sorts in how different libraries/frameworks handle state. Allow me to lay some context to hopefully help you understand what the constraints are and why, and then we can see if a solution is possible.

The immutable way
On one end of the spectrum, you have the FP approach where things are typically immutable. To change something, you clone it and put new values on it. Detecting changes happens via diffing the old and the new object. Reconciliation.
This is how immerjs works, this is how React works, and a few others.

The advantage is that it can be easier to reason about state. You can assume an object doesnā€™t change after itā€™s creation. (as for whether this is really an advantageā€¦ for a library like ImmerJS, sure, but not for UI programming which is all about mutable interactive components - we chose our design very consciously)

The disadvantage is that itā€™s slow and requires more boilerplate. This is why Aurelia runs circles around React and why Aurelia apps tend to be easier to digest. (sorry, I had to.)

The mutable way
Of course, weā€™re not perfect either, nothing is. Now youā€™ve got these pesky getters to deal with that change your objects and make them hard to clone.

On the other end of the spectrum, you have the OOP approach where things are typically mutable. To change something, you, well, change it. And something sits in between to observe that.

Now, FP and OOP both have a time and place. I am not for OOP and against FP or vice versa. I use both, and combine them, etc.

However, when dealing with different 3rd party libraries, you canā€™t flat-out mix the two approaches because they truly need to be designed for compatibility with the other approach.

The solution
ImmerJS claims they cannot work with getters. Itā€™s not that it canā€™t be done, but they donā€™t know what assumptions they can make. And they donā€™t want to risk making the wrong ones, making their library flaky! Makes perfect sense.

So, we need something custom made here. It doesnā€™t have to be big, hard, complex, but it needs to be written by us so that we can code it according to assumptions that are safe to make within the Aurelia framework.

Since this is a problem that wonā€™t go away in vNext, and the observation system is still identical in concept, I see value in making an official solution for a safe deep clone of an observed object.
I think a core library for doing that is probably the best way to go. There are more needs for it than just this particular integration anyway.

Iā€™m estimating a few dozen LOC at most. Maybe 100-200 if we want to be very thorough.

What do you think @zewa666 ?

1 Like
13

I actually feared you would say that @fkleuver. I havent nearly looked deeply enough into the Details of Immerjs implementation to judge what it takes to fix up their lib. A custom Proxy cloner is a solution I just wonder whether itā€™d make more sense to contribute back to Immer, perhaps with an extension specifically for Aurelia

1 Like
14

I actually got the impression that immerjs was doing more than just immutable observation. If thatā€™s all it does, we might as well offer the concept as a core feature. We practically already have most of what immer does in vNext scattered around the place for our proxy based observation alternative.

1 Like
15

@Jenselme and @swalters can I ask you for one thing to try out? In your sample modify immer.js source (most likely dist/immer.module.js) and search for the line throw new Error("Immer drafts cannot have computed properties");

Now right before that this:

if (base.__observers__ && base.__observers__.hasOwnProperty(key)) {
  return;
}

Let me know whether this still keeps the error around.

1 Like
16

Since the idea of being able to skip certain keys (not only getters/setters) might be of general interest, Iā€™ve created an issue in the immer repo

3 Likes
17

When I encountered the issue, I dig into immer and I think itā€™s more or less here that should be handled. If I remember correctly, thereā€™s a boolean switch we can pass to the function to make it work. Iā€™ll try to test soon.

1 Like
18

@zewa666 That seems to work . Iā€™m still going through a lot of testing with different scenarios to ensure Iā€™m on the right track.

Iā€™m attempting to refactor an extremely large app into the store pattern. Itā€™s painful but I like the simplicity Iā€™m seeing. Our app logic has gotten out of control and it seems that Aurelia store can bring it back under control. This is proving hard to do. Itā€™s even more of a rewrite then I imagined because we two-way bind everywhere.

If immer wonā€™t resolve the issue you created, do you think it would make sense to fork immer and create an Aurelia Store specific version? (update: I see thatā€™s suggested above by @fkleuver)

2 Likes
19

Yep switching to the Store pattern is time consuming. But essentially most time is spent to untangle the spaghetti of service interdependencies. I did this now for two old apps and started building our Testing IDE Webtestit with the plugin. Its a meanwhile pretty massive App and the Store has proven as the way to manage chaos so far.

Talking about the App we often need some special addons for certain dependencies like now for immer. So what we do is create a PR and while that gets merged we make use of https://github.com/ds300/patch-package. The best thing since monkey patching. So I doubt a full fork is needed but instead a simple patch might do it.

Please let me know what your tests say about the proposed fix so we can push that further. As for vNext though, as @fkleuver mentioned, there could be a good chance we implement something Aurelia specific for ourselves. Before doing so Iā€™d just like to know more about the use cases

1 Like
20

btw @swalters, thinking about your two-way refactoring issue I had an yet untested idea.

if you have lets say something like this in your view

<input type="text" value.bind="state.user.firstName">

and you want the value modified not to be applied to the state object in order to avoid mutation. In that case you could try to create a custom valueconverter (lets call it stop) to do the job for you. Above becomes:

<input type="text" value.bind="state.user.firstName | stop: 'user.firstName'">

So you pass in a dot-separated-string the path to the bound property of the state.

The ValueConverter could be something along these lines:

import {get, set} from "lodash";

function genericUpdateAction(state: State, key: string, value: any) {
  // use immer or object spreads or lodash set on a clone
 // e.g 
  const clone = deepClone(state);
  return _.set(clone, key, value); 
}

@autoinject
export class StopValueConverter {
  private state: State;
  private subscription: Subscription;

  constructor(private store: Store<State>) {
    this.subscription = this.store.state.subscribe(state => this.state = state);
  }

  fromView(value, key) {
    const originalData = _.get(this.state, key);

    // fire the generic state update action
    this.store.dispatch(genericUpdateAction, key, value);

    // return the original data, thus negating the effect of two-way writes
    return originalData;
  }

  unbind() {
    this.subscription.unsubscribe();
  }
}

As said this is pretty much untested (not on my PC so no IDE available :-o), but perhaps of help.

2 Likes