[Discussion] Hide all private properties and functions in constructor closure for classes / implementations

[MSNev]

• [X] This only affects the JavaScript OpenTelemetry library
• [ ] This may affect other libraries, but I would like to get opinions here first

To avoid unexpected / undocumented usages of internal properties we should avoid exposing them directly on the class as private properties and functions are still defined on the resulting JS object and therefore available for anyone to use (ignoring typescript compile time checks)

Example

export class ContextAPI {
  private static _instance?: ContextAPI;    // <-- Singletons are problematic

  /** Empty private constructor prevents end users from constructing a new instance of the API */
  private constructor() {} // <-- required

  /** Get the singleton instance of the Context API */
  public static getInstance(): ContextAPI {
    ...
  }

  /** Set the current context manager. Returns the initialized context manager */
  public setGlobalContextManager(contextManager: ContextManager): ContextManager {
    ...
  }

  /** Get the currently active context */
  public active(): Context {
    return this._getContextManager().active();
  }

    ...

  private _getContextManager(): ContextManager {
    return (
      _global[GLOBAL_CONTEXT_MANAGER_API_KEY]?.(
        API_BACKWARDS_COMPATIBILITY_VERSION
      ) ?? NOOP_CONTEXT_MANAGER
    );
  }

Could become something like

export class ContextAPI {
  /** Set the current context manager. Returns the initialized context manager */
  public setGlobalContextManager: (contextManager: ContextManager) => ContextManager;

  /** Get the currently active context */
  public active: () => Context;

  private constructor() {
    let _self = this;
    function _getContextManager(): ContextManager {
        return (
          _global[GLOBAL_CONTEXT_MANAGER_API_KEY]?.(
            API_BACKWARDS_COMPATIBILITY_VERSION
          ) ?? NOOP_CONTEXT_MANAGER
        );
    }

    _self.setGlobalContextManager = (contextManager: ContextManager) => ContextManager {
        ...
    };
    _self.active = () => _getContextManager().active();
 }

  /** Get the singleton instance of the Context API */
  public static getInstance(): ContextAPI {
    // Use _global instead??
    ...
  }
    ...
}

I'm not suggesting that we use or take a dependency on the following component I'm just including it here as I documented the issues quite a bit as part of creating the component https://github.com/microsoft/DynamicProto-JS.

This can happen after GA as long as we are sure that the private scoped functions / properties are defined correctly -- to avoid "known" breaking issues as it would not stop people taking indirect dependencies on anything private. *

• = Assuming that this is not done on a Base class as the above is effectively converting the functions from being defined on the prototype to being defined on the instance and that is where breaking changes can (and do) occur -- And the primary thing that the DynamicProto helper resolves

[dyladan]

Discussed at meeting. Defining functions as instance variables provides most of the benefit.