mardi 20 septembre 2022

General questions on: Ember DDAU, Glimmer components (this.args), JavaScript getters and encapsulation

I work on an Ember team using ES5, Octane and Glimmer and am generally happy with the tech stack. I've written JavaScript webapps for about 12 years, doing fullstack work before that. I'm reviewing a bunch of code that somehow got merged into our solution (partially my fault because I didn't review the related PRs before they were merged) and am trying to settle an argument with the dev who wrote it.

This dev - who as an experienced dev should know better - built a whole section of our app that repeatedly does the following:

  1. Ember route gets this.model from the API and passes it as @model to a controller
  2. Ember controller passes @model to a child component as @model
  3. Ember component uses a JavaScript getter to create a "mask" or alias for part of the model: get localObject() { return this.args.model.parentObject; } and passes that getter (as this.localObject) to another child component. This getter is really just an alias because it does not apply any logic, it just makes typing the code faster for the dev.
  4. The second (or third or forth) child component receives the "masked" model as this.args.maskedModel, then uses another getter to mask that: get localModel() { return this.args.maskedModel; }, then the "final" child component directly mutates the passed-in property by calling set on it: set(this.localModel, 'someSubProperty', true);

Right now, the dev who used this antipattern ALL OVER the work they wrote is arguing that since this "works" that it's fine and it doesn't need to be refactored. I'm arguing this pattern breaks encapsulation, DDAU, misuses JavaScript getters and somehow forces two-way data binding across them and Glimmer's this.args object.

Personally, I don't even understand how this is working, since JavaScript getters and Glimmer's this.args object are both supposed to only allow one-way data binding. But somehow, when the child component mutates the passed-in objects from the parent (reaching across TWO different getters), the parent magically gets the updated object. Instead of directly mutating the passed-in objects, the child component should call a passed-in closure action from the parent and allow the parent to mutate the data.

I also think it is a misuse of JavaScript getters to use them as an alias to make referring to this.args.model.someObject easier (less typing), without applying any logic in the getter function. Ember has a computed property "alias" for this purpose, though I still think "aliasing" this.args.something in a component is a bad idea because it gives a dev the false notion they can directly mutate the alias, if they don't realize it is referring to a passed-in value from the parent.

I think all this bad code directly breaks Data Down, Actions Up in Ember, as well as the long-standing computer science principle of data encapsulation.

Does anyone know why this is working since ES5 getters and Glimmer's this.args object are supposed to be one-way and immutable? Is it a flaw in Ember/Glimmer? We are on Ember 3.28 now, is this fixed in a newer version? How can I explain to this developer, who is being stubborn on this, that this is a terrible way to write code??




Aucun commentaire:

Enregistrer un commentaire