Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Deep cloning objects with private fields #129

Closed
JorisBlanken opened this issue Sep 12, 2018 · 8 comments
Closed

Deep cloning objects with private fields #129

JorisBlanken opened this issue Sep 12, 2018 · 8 comments

Comments

@JorisBlanken
Copy link

A common method of deep cloning objects and therefore also class instances uses the Object.assign function.

class Example {
    #x = 0;
    get x(){ return this.#x; }
    set x(n){ this.#x = n; }
}

let a = new Example();
let b = {};
Object.assign(b, a);

console.log(b) // prints "{}"

With the implementation of true private fields that are inaccessible even with reflection, how can deep cloning be accomplished?

@ljharb
Copy link
Member

ljharb commented Sep 12, 2018

It can't, and that's fine, because deep cloning is a) a code smell, and b) likely impossible to do generically in JS (see https://twitter.com/bakkoting/status/1039364346525020160 for a reference).

You could make a method on Example itself that can provide a full copy, but Object.assign is not a "deep cloning" nor even a "cloning" mechanism.

@JorisBlanken
Copy link
Author

I agree that deep cloning itself can be considered bad practice and that it would be best suited to provide a function on the class to make "cloning" possible.

However I'd like to consider that not everyone has the liberty of working with code to which such additions can be made. Sometimes accessing object members which should not be accessed is the only way to accomplish something with the current setup, sometimes hacks are the only way to accomplish something (considering external constraints).

I think it would be sufficient to have at least one method of accessing private member variables, making clear that the user knows what they're doing.

@bakkot
Copy link
Contributor

bakkot commented Sep 12, 2018

The reflection vs encapsulation debate has been had at great length (for example here). We came down on the side of encapsulation. See FAQ.

We recognize the tradeoffs, but still think, on consideration, that true privacy is worth it.


In any case, there is no reflection which allows you to get at closed-over variables. Code like

function Factory() {
  var x = 0;
  return {
    foo() {
      return x++;
    },
  };
}

creates objects which cannot be cloned. So this feature doesn't do much to change that.

@atombender
Copy link

While the above arguments are fine, I'm not sure how you can implement immutable model objects using private properties. For example:

class Toy {
  #color = "blue";

  constructor(color) {
    this.#color = color;
  }

  get color() {
    return this.#color;
  }

  setColor(value) {
    return new Toy(value);
  }
}

The above is okay if you have 2-3 private properties, but if you have many — 10? 20? — it starts becoming unwieldly, especially as positional arguments in a constructor are not a great idea:

constructor(color, width, height, depth, weight, material) {
  [...]

Accepting an object is a potential solution:

constructor(props = {}) {
  this.#color = props.color;
  [...]
}

but that has downsides (no "compile-time" checking, especially in TypeScript), and doesn't solve the fact that setters still end up looking like so:

setColor(value) {
  return new Toy({
    color: value,
    width: this.#width,
    height: this.#height,
    depth: this.#depth,
    weight: this.#weight,
    material: this.#material
  });
}

One could sitestep this somehow:

setColor(value) {
  return new Toy(Object.assign(this.asObject{), {color: value}));
}

asObject() {
  return {color: this.#color, width: this.#width, height: this.#height,
    depth: this.#depth, category: this.#category, material: this.#material};
}

but it's still brittle, and not pretty. Right now, it seems the best bet for immutable objects is something like:

import {Map} from 'immutable';

class Toy {
  #props = new Map();

  constructor(props) {
    this.#props = props;
  }

  get color() {
    return this.#props.get('color');
  }

  setColor(value) {
    return new Toy(this.#props.set('color', value));
  }
}

Immutable also comes with a Record helper to do something very similar.

Is there some mechanism for working around the above problems?

@littledan
Copy link
Member

Better ergonomics for immutable objects in general, and improved facilities for cloning (we can admonish developers all we want but it's clearly important to many) seem to me like important things to investigate in the near future. If you want to define an object that you can clone with Object.assign, JSON.parse/stringing, or structured clone, please continue using ordinary object literals for now, rather than private fields, as we haven't worked this out yet.

@slikts
Copy link

slikts commented Dec 6, 2018

The examples trying to use private fields for immutability are contrived and can just be done with Object.freeze():

class Toy {
  constructor(props) {
    Object.assign(this, props);
    Object.freeze(this);
  }

  setColor(color) {
    return new Toy({
        ...this,
        color,
    });
  }
}

This is, of course, inefficient; a proper solution would use structural sharing, but that's not really relevant to privacy.

@atombender
Copy link

atombender commented Dec 6, 2018 via email

@hax
Copy link
Member

hax commented Dec 13, 2018

Object.freeze() is just shallow, and it only solve a special case, can not deny the requirements of enumeration.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants