Implement privatize, private fields on prototypes

[tshemsedinov]

Function moved from impress and needed for

• code is properly formatted (npm run fmt)
• tests are added/updated
• documentation is updated (npm run doc to regenerate documentation based on comments)
• description of changes is added under the Unreleased header in CHANGELOG.md

[DScheglov]

Nested if doesn't cry about "let's keep constants".
I guess the loop body could be refactored to:

const field = instance[fieldName];
const isMethod = typeof field === 'function';
if (!isMethod && !isConstantName(fieldName)) continue; // skipping private fields
iface[fieldName] = isMethod ? field.bind(instance) : field;

[DScheglov]

Later Object.freeze(iface); covers this property definition.

[DScheglov]

what about private methods?

const dest = privatize({
  _privateMethod() {},
  publicMethod() { this._privateMethod(); },
});

Many developers could expect that the dest contains only publicMethod.
So, I suggest to extract logic of private fields recogintion to the dedicated function and pass it to the privatize as a parameter.

const ASCII_A = 'A'.charCodeAt(0);
const ASCII_Z = 'Z'.charCodeAt(0);

const isExplicitlyPrivate = name => name.charAt(0) === '_';

const isMethod = value => typeof value === 'function';

const isConstantName = name => {
  if (name !== name.toUpperCase()) return false;
  const firstCharCode = name.charCodeAt(0);
  return firstCharCode >= ASCII_A && firstCharCode <= ASCII_Z;
};

const isPublicField = (name, value) =>
  !isExplicitlyPrivate(name) &&
  (isMethod(value) || isConstantName(name));

const isPrivateField = (name, value) => !isPublicField(name, value);

const privatize = (instance, isPrivate = isPrivateField) => {
  const iface = {};
  const fields = Object.keys(instance);
  for (const fieldName of fields) {
    const fieldValue = instance[fieldName];

    if (isPrivate(fieldName, fieldValue)) continue; // skipping private fields

    iface[fieldName] = isMethod(fieldValue)
      ? fieldValue.bind(instance)
      : fieldValue;
  }
  return Object.freeze(iface);
};

[belochub]

Suggested change

	test.strictSame(err.constructor.name, 'TypeError');
	test.isError(err, new TypeError());

[belochub]

Suggested change

	destination.CONSTANT = 'can not change freezed';
	destination.CONSTANT = 'cannot change frozen';

[belochub]

Instead of repeating the same check twice, I'd propose to catch the error and save it into a variable that can be asserted outside of the try-catch block, and then keep a check for the value of the field.

let err = null;
try {
  destination.CONSTANT = 'cannot change frozen';
} catch (e) {
  err = e;
}
test.isError(err, new TypeError());
test.strictSame(destination.CONSTANT, 'constant value');

Though a simpler way of testing the same thing would be to call Object.getOwnPropertyDescriptor() for that property, and test that descriptor object doesn't have writable or configurable set to true and has set set to undefined.

[belochub]

Ditto here, continuing on the proposal in the comment above, here we can first assert that Object.getOwnPropertyDescriptor() returns undefined and then check the result of calling Object.isExtensible() which is expected to be false, meaning that this field does not exist and it cannot be added.

[belochub]

Ditto.

[belochub]

I don't think that this check is safe enough if you only wanted to limit this to uppercase Latin alphabet since the basic ASCII punctuation and symbols, such as '[', '\', ']', '^', '_', '`' will not change when put into uppercase and have their code value higher than the code value of 'Z', thus keys containing one of those characters in the starting position might be incorrectly mapped. I would propose to clearly specify the ends of the range of the characters that such keys are supposed to start with.