[beta.55] private fields transform produces unnecessary WeakMaps, and not-private output.

[trusktr]

Bug Report

I played around with the new private fields in the repl and discovered some things:

• The fields aren't truly private. The output code can be imported by script tag (for example) and it leaks the private fields into public space.
  • They are only "private" strictly to the Babel output code.
  • See this codepen example of public code accessing private fields.
  • The _foo and _bar variables shouldn't be accessible. Babel should expose only the variables explicitly defined in the original source.
• Unnecessary number of WeakMaps are created, one for each field.
  • If the fields aren't truly private, there's no need to create WeakMaps at all.
• We don't know where users will stick output code (f.e. a <script> tag, not necessarily a closure), so output should be truly-private, otherwise the performance hit of making WeakMaps is entirely defeated.
  • In many cases people might use Node require or Webpack, which automatically wraps code in a function thus creating encapsulation, but this isn't always the case.

See my truly-private implementation idea: babel/proposals#12 (comment)

Please feel free to borrow any ideas from there.

[babel-bot]

Hey @trusktr! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

[ljharb]

This seems a more general problem, unrelated to private fields - if the file is output as a Script, it should be inside an IIFE - otherwise it will create a bunch of global variables.

[ljharb]

However, I also don't think anyone is (or should be) sticking babel output directly in a script tag - a bundler is required for modern web dev, and I don't think we need to optimize for people using last-decade practices.

[trusktr]

An ideal world it will be when everyone is using modules! I myself use script tags to prototype things, exposing globals. It's just easy.

[ljharb]

"easy" and "safe" (and "easy" and "worth babel worrying about") aren't the same thing :-)

[loganfsmyth]

Babel 100% assumes that you're using modules at the moment, both in that it creates top-level declarations that are expected not to leak, and that it often generates variable names that will collide across scripts. I'm all for exploring options for resolving that, I don't think it's particularly tied to private fields.

[trusktr]

@loganfsmyth Putting the encapsulation concern aside, there's the other issues (f.e. one WeakMap per private prop is excessive).

(I still think it is unfair to assume everyone will use Babel output inside a closure. But, most people will.)

[loganfsmyth]

Is size of output code the primary concern? It seems like the number of WeakMaps is just an implementation detail.

[ishitatsuyuki]

What about using a Symbol as the accessing key? It's properly scoped, minifier friendly, and has no performance impact.

[ljharb]

@ishitatsuyuki symbols aren't private, in any way, shape, or form.

[trusktr]

Is size of output code the primary concern? It seems like the number of WeakMaps is just an implementation detail.

@loganfsmyth Code size isn't the concern, it's more about resources (mostly memory use): there's too many WeakMaps at the moment. This could be implemented with just 1 WeakMap per module no matter how many classes are inside the module, instead of one for every private property of every class.

[trusktr]

@ishitatsuyuki symbols aren't private, in any way, shape, or form.

I agree, they aren't completely private, but neither are the current WeakMap privates in all cases.

I'm sure there's plenty of projects still concatenating files with Gulp or Grunt without the use of modules and using Babel for language features.

Gulp and Grunt together get just less than 1 million downloads per week. That's still a considerable number of projects that are likely not to be using modules!

But anyway, yeah, move to modules if possible!

[loganfsmyth]

The current is wasteful on resources.

Do you have statistics to back that up? I generally ignore any claims about resources and performance in JS unless you've actually measured and compared on various engines.

I don't really disagree that it's an option, I just disagree that it realistically makes any difference for 99% of cases, especially with no numbers to back up your points.

[ishitatsuyuki]

https://jsperf.com/weakmap-vs-property/1

[loganfsmyth]

@ishitatsuyuki That's not quite the same. I'm 100% behind a version of private fields that uses name-mangling instead of WeakMaps in order to improve performance by using real props (once we've gotten the WeakMap implementation landed anyway), but @trusktr seems focused on the separation between a single WeakMap vs many, and I'm not as convinced that that one vs many makes much of a difference.

[nicolo-ribaudo]

@ishitatsuyuki We have loose mode that does exactly this 🙂

[trusktr]

once we've gotten the WeakMap implementation landed anyway

Well, good to just get it working first anyway!

Do you have statistics to back that up?

No runtime tests, but here's some numbers:

Currently: 100 classes each with 100 private properties in 100 modules (1 class per file) means 10,000 WeakMaps (because 100 classes x 100 properties, regardless of the number of modules). Each private property of each class instance needs 1 object for the property metadata, so if we have 1 instance of each class, that's 10,000 more objects, for 20,000 total.

With one-weakmap-per-module: For the same setup as the previous 100 classes/files, there will be 100 WeakMaps (one per file/module). Each WeakMap will need one cache object per class instance. This will be 100 cache objects if there is 1 instance of each class. In each cache object, there will still need to be the same number of private property metadata objects, which is 10,000. The total is 10,200.

If we disregard the property metadata objects count which remains the same in both methods, we have 10,000 vs 200.

---

If we spread those classes across 50 files instead of 100 (for example), this leaves the current number of WeakMaps the same, but with weakmap-per-module we now have 100 objects instead of 200.

---

For this input,

class Foo {
  #foo = 5
  #bar = 6
  test() {
    console.log(this.#foo, this.#bar)
  }
}

const f = new Foo
f.test()

the output would be more like

"use strict";

function _instanceof(left, right) { if (right != null && typeof Symbol !== "undefined" && right[Symbol.hasInstance]) { return right[Symbol.hasInstance](left); } else { return left instanceof right; } }

function _classCallCheck(instance, Constructor) { if (!_instanceof(instance, Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

function _defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } }

function _createClass(Constructor, protoProps, staticProps) { if (protoProps) _defineProperties(Constructor.prototype, protoProps); if (staticProps) _defineProperties(Constructor, staticProps); return Constructor; }

var _privatesMap = new WeakMap // the single WeakMap in the module

function _private(obj) {
  var privates = _privatesMap.get(this);
  if (!privates) _privatesMap.set(this, privates = {});
  return privates;
}

var Foo =
/*#__PURE__*/
function () {
  function Foo() {
    _classCallCheck(this, Foo);

    _private(this).foo = {
      writable: true,
      value: 5
    };

    _private(this).bar = {
      writable: true,
      value: 6
    };
  }

  _createClass(Foo, [{
    key: "test",
    value: function test() {
      console.log(_private(this).foo.value, _private(this).bar.value);
    }
  }]);

  return Foo;
}();

[ljharb]

How many classes have 100 properties in their entire prototype chain, let alone have 100 own public properties, let alone will have 100 private properties? I'm not sure that's a realistic test case.