Replace mutable export with getter function

Mutable exports can create hard to understand and debug code, in particular with re-exports across multiple modules. Change-Id: I4a66839c4fe54a4c14335d847d9d78fccf31db33
2020-08-04 11:45:54 +02:00
parent 2c35afe878
commit 49c2b9c5a3
13 changed files with 44 additions and 38 deletions
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
@@ -33,7 +33,7 @@ import {htmlTemplate} from './gr-change-list-item_html.js';
 import {ChangeTableMixin} from '../../../mixins/gr-change-table-mixin/gr-change-table-mixin.js';
 import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
 import {getDisplayName} from '../../../utils/display-name-util.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
 import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
 import {appContext} from '../../../services/app-context.js';
 import {truncatePath} from '../../../utils/path-list-util.js';
@@ -107,7 +107,7 @@ class GrChangeListItem extends ChangeTableMixin(GestureEventListeners(
  attached() {
    super.attached();
    pluginLoader.awaitPluginsLoaded().then(() => {
-      this._dynamicCellEndpoints = pluginEndpoints.getDynamicEndpoints(
+      this._dynamicCellEndpoints = getPluginEndpoints().getDynamicEndpoints(
          'change-list-item-cell');
    });
  }
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -31,7 +31,7 @@ import {appContext} from '../../../services/app-context.js';
 import {ChangeTableMixin} from '../../../mixins/gr-change-table-mixin/gr-change-table-mixin.js';
 import {KeyboardShortcutMixin, Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
 import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
 import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
 import {changeIsOpen} from '../../../utils/change-util.js';

@@ -168,7 +168,7 @@ class GrChangeList extends ChangeTableMixin(
  attached() {
    super.attached();
    pluginLoader.awaitPluginsLoaded().then(() => {
-      this._dynamicHeaderEndpoints = pluginEndpoints.getDynamicEndpoints(
+      this._dynamicHeaderEndpoints = getPluginEndpoints().getDynamicEndpoints(
          'change-list-header');
    });
  }
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -55,7 +55,7 @@ import {GrEditConstants} from '../../edit/gr-edit-constants.js';
 import {GrCountStringFormatter} from '../../shared/gr-count-string-formatter/gr-count-string-formatter.js';
 import {getComputedStyleValue} from '../../../utils/dom-util.js';
 import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
 import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
 import {RevisionInfo} from '../../shared/revision-info/revision-info.js';

@@ -501,9 +501,9 @@ class GrChangeView extends KeyboardShortcutMixin(
    pluginLoader.awaitPluginsLoaded()
        .then(() => {
          this._dynamicTabHeaderEndpoints =
-            pluginEndpoints.getDynamicEndpoints('change-view-tab-header');
+            getPluginEndpoints().getDynamicEndpoints('change-view-tab-header');
          this._dynamicTabContentEndpoints =
-            pluginEndpoints.getDynamicEndpoints('change-view-tab-content');
+            getPluginEndpoints().getDynamicEndpoints('change-view-tab-content');
          if (this._dynamicTabContentEndpoints.length !==
          this._dynamicTabHeaderEndpoints.length) {
            console.warn('Different number of tab headers and tab content.');
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -37,7 +37,7 @@ import {KeyboardShortcutMixin, Shortcut} from '../../../mixins/keyboard-shortcut
 import {GrFileListConstants} from '../gr-file-list-constants.js';
 import {GrCountStringFormatter} from '../../shared/gr-count-string-formatter/gr-count-string-formatter.js';
 import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
 import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
 import {appContext} from '../../../services/app-context.js';
 import {SpecialFilePath} from '../../../constants/constants.js';
@@ -311,15 +311,15 @@ class GrFileList extends KeyboardShortcutMixin(
  attached() {
    super.attached();
    pluginLoader.awaitPluginsLoaded().then(() => {
-      this._dynamicHeaderEndpoints = pluginEndpoints
+      this._dynamicHeaderEndpoints = getPluginEndpoints()
          .getDynamicEndpoints('change-view-file-list-header');
-      this._dynamicContentEndpoints = pluginEndpoints
+      this._dynamicContentEndpoints = getPluginEndpoints()
          .getDynamicEndpoints('change-view-file-list-content');
-      this._dynamicPrependedHeaderEndpoints = pluginEndpoints
+      this._dynamicPrependedHeaderEndpoints = getPluginEndpoints()
          .getDynamicEndpoints('change-view-file-list-header-prepend');
-      this._dynamicPrependedContentEndpoints = pluginEndpoints
+      this._dynamicPrependedContentEndpoints = getPluginEndpoints()
          .getDynamicEndpoints('change-view-file-list-content-prepend');
-      this._dynamicSummaryEndpoints = pluginEndpoints
+      this._dynamicSummaryEndpoints = getPluginEndpoints()
          .getDynamicEndpoints('change-view-file-list-summary');

      if (this._dynamicHeaderEndpoints.length !==
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
@@ -28,7 +28,7 @@ import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
 import {ChangeStatus} from '../../../constants/constants.js';
 import {patchNumEquals} from '../../../utils/patch-set-util.js';
 import {changeIsOpen} from '../../../utils/change-util.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';

 /**
 * @extends PolymerElement
@@ -372,7 +372,7 @@ class GrRelatedChangesList extends GestureEventListeners(
    // to check for that and stay visible if we find any such visible content.
    // (We consider plugins visible except if it's main element has the hidden
    // attribute set to true.)
-    const plugins = pluginEndpoints.getDetails('related-changes-section');
+    const plugins = getPluginEndpoints().getDetails('related-changes-section');
    this.hidden = !(plugins.some(plugin => (
      (!plugin.domHook)
        || plugin.domHook.getAllAttached().some(
--- a/polygerrit-ui/app/elements/gr-app-global-var-init.js
+++ b/polygerrit-ui/app/elements/gr-app-global-var-init.js
@@ -41,7 +41,7 @@ import {GrEtagDecorator} from './shared/gr-rest-api-interface/gr-etag-decorator.
 import {GrThemeApi} from './plugins/gr-theme-api/gr-theme-api.js';
 import {SiteBasedCache, FetchPromisesCache, GrRestApiHelper} from './shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js';
 import {GrLinkTextParser} from './shared/gr-linked-text/link-text-parser.js';
-import {pluginEndpoints, GrPluginEndpoints} from './shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints, GrPluginEndpoints} from './shared/gr-js-api-interface/gr-plugin-endpoints.js';
 import {GrReviewerUpdatesParser} from './shared/gr-rest-api-interface/gr-reviewer-updates-parser.js';
 import {GrPopupInterface} from './plugins/gr-popup-interface/gr-popup-interface.js';
 import {GrRangeNormalizer} from './diff/gr-diff-highlight/gr-range-normalizer.js';
@@ -139,7 +139,8 @@ export function initGlobalVariables() {
  window.Gerrit.Auth = appContext.authService;

  window.Gerrit._pluginLoader = pluginLoader;
-  window.Gerrit._endpoints = pluginEndpoints;
+  // TODO: should define as a getter
+  window.Gerrit._endpoints = getPluginEndpoints();

  window.Gerrit.slotToContent = slot => slot;
  window.Gerrit.rangesEqual = rangesEqual;
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
@@ -20,7 +20,7 @@ import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-l
 import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
 import {PolymerElement} from '@polymer/polymer/polymer-element.js';
 import {htmlTemplate} from './gr-endpoint-decorator_html.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
 import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';

 const INIT_PROPERTIES_TIMEOUT_MS = 10000;
@@ -61,7 +61,7 @@ class GrEndpointDecorator extends GestureEventListeners(
    for (const [el, domHook] of this._domHooks) {
      domHook.handleInstanceDetached(el);
    }
-    pluginEndpoints.onDetachedEndpoint(this.name, this._endpointCallBack);
+    getPluginEndpoints().onDetachedEndpoint(this.name, this._endpointCallBack);
  }

  _initDecoration(name, plugin, slot) {
@@ -161,12 +161,12 @@ class GrEndpointDecorator extends GestureEventListeners(
  ready() {
    super.ready();
    this._endpointCallBack = this._initModule.bind(this);
-    pluginEndpoints.onNewEndpoint(this.name, this._endpointCallBack);
+    getPluginEndpoints().onNewEndpoint(this.name, this._endpointCallBack);
    if (this.name) {
      pluginLoader.awaitPluginsLoaded()
-          .then(() => pluginEndpoints.getAndImportPlugins(this.name))
+          .then(() => getPluginEndpoints().getAndImportPlugins(this.name))
          .then(() =>
-            pluginEndpoints
+            getPluginEndpoints()
                .getDetails(this.name)
                .forEach(this._initModule, this)
          );
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.js
@@ -23,7 +23,7 @@ import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
 import {html} from '@polymer/polymer/lib/utils/html-tag.js';
 import {resetPlugins} from '../../../test/test-utils.js';
 import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
 import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';

 const pluginApi = _testOnly_initGerritPluginApi();
@@ -57,7 +57,7 @@ suite('gr-endpoint-decorator', () => {
  setup(done => {
    resetPlugins();
    container = basicFixture.instantiate();
-    sinon.stub(pluginEndpoints, 'importUrl')
+    sinon.stub(getPluginEndpoints(), 'importUrl')
        .callsFake( url => Promise.resolve());
    pluginApi.install(p => plugin = p, '0.1',
        'http://some/plugin/url.html');
@@ -84,7 +84,7 @@ suite('gr-endpoint-decorator', () => {
    const endpoints =
        Array.from(container.querySelectorAll('gr-endpoint-decorator'));
    assert.equal(endpoints.length, 3);
-    assert.isTrue(pluginEndpoints.importUrl.calledWith(
+    assert.isTrue(getPluginEndpoints().importUrl.calledWith(
        new URL('http://some/plugin/url.html')
    ));
  });
--- a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
+++ b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
@@ -20,7 +20,7 @@ import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-l
 import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
 import {PolymerElement} from '@polymer/polymer/polymer-element.js';
 import {htmlTemplate} from './gr-external-style_html.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
 import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';

 /** @extends PolymerElement */
@@ -57,9 +57,9 @@ class GrExternalStyle extends GestureEventListeners(
  }

  _importAndApply() {
-    pluginEndpoints.getAndImportPlugins(this.name)
+    getPluginEndpoints().getAndImportPlugins(this.name)
        .then(() => {
-          const moduleNames = pluginEndpoints.getModules(this.name);
+          const moduleNames = getPluginEndpoints().getModules(this.name);
          for (const name of moduleNames) {
            this._applyStyle(name);
          }
--- a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.js
@@ -19,7 +19,7 @@ import '../../../test/common-test-setup-karma.js';
 import {resetPlugins} from '../../../test/test-utils.js';
 import './gr-external-style.js';
 import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
-import {pluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
+import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
 import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
 import {html} from '@polymer/polymer/lib/utils/html-tag.js';

@@ -66,7 +66,7 @@ suite('gr-external-style integration tests', () => {
  };

  setup(() => {
-    sinon.stub(pluginEndpoints, 'importUrl')
+    sinon.stub(getPluginEndpoints(), 'importUrl')
        .callsFake( url => Promise.resolve());
    sinon.stub(pluginLoader, 'awaitPluginsLoaded')
        .returns(Promise.resolve());
@@ -81,7 +81,7 @@ suite('gr-external-style integration tests', () => {
  test('imports plugin-provided module', async () => {
    lateRegister();
    await new Promise(flush);
-    assert.isTrue(pluginEndpoints.importUrl.calledWith(new URL(TEST_URL)));
+    assert.isTrue(getPluginEndpoints().importUrl.calledWith(new URL(TEST_URL)));
  });

  test('applies plugin-provided styles', async () => {
@@ -96,7 +96,7 @@ suite('gr-external-style integration tests', () => {
    plugin.registerStyleModule('foo', 'some-module');
    await new Promise(flush);
    // since loaded, should not call again
-    assert.isFalse(pluginEndpoints.importUrl.calledOnce);
+    assert.isFalse(getPluginEndpoints().importUrl.calledOnce);
  });

  test('does not double apply', async () => {
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-endpoints.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-endpoints.ts
@@ -217,7 +217,12 @@ export class GrPluginEndpoints {
 }

 // TODO(dmfilippov): Convert to service and add to appContext
-export let pluginEndpoints = new GrPluginEndpoints();
+let pluginEndpoints = new GrPluginEndpoints();
+
+// To avoid mutable-exports, we don't want to export above variable directly
+export function getPluginEndpoints() {
+  return pluginEndpoints;
+}
 export function _testOnly_resetEndpoints() {
  pluginEndpoints = new GrPluginEndpoints();
 }
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.js
@@ -24,7 +24,7 @@ import {
 } from './gr-api-utils.js';
 import {Plugin} from './gr-public-js-api.js';
 import {getBaseUrl} from '../../../utils/url-util.js';
-import {pluginEndpoints} from './gr-plugin-endpoints.js';
+import {getPluginEndpoints} from './gr-plugin-endpoints.js';

 /**
 * @enum {string}
@@ -210,7 +210,7 @@ export class PluginLoader {

  _checkIfCompleted() {
    if (this.arePluginsLoaded()) {
-      pluginEndpoints.setPluginsReady();
+      getPluginEndpoints().setPluginsReady();
      if (this._loadingResolver) {
        this._loadingResolver();
        this._loadingResolver = null;
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
@@ -32,7 +32,7 @@ import {GrRepoApi} from '../../plugins/gr-repo-api/gr-repo-api.js';
 import {GrSettingsApi} from '../../plugins/gr-settings-api/gr-settings-api.js';
 import {GrStylesApi} from '../../plugins/gr-styles-api/gr-styles-api.js';
 import {GrPluginActionContext} from './gr-plugin-action-context.js';
-import {pluginEndpoints} from './gr-plugin-endpoints.js';
+import {getPluginEndpoints} from './gr-plugin-endpoints.js';

 import {
  PRELOADED_PROTOCOL,
@@ -92,7 +92,7 @@ export class Plugin {
  }

  registerStyleModule(endpoint, moduleName) {
-    pluginEndpoints.registerModule(this, {
+    getPluginEndpoints().registerModule(this, {
      endpoint,
      type: EndpointType.STYLE,
      moduleName,
@@ -139,7 +139,7 @@ export class Plugin {
    const slot = (opt_options && opt_options.slot) || '';
    const domHook = this._domHooks.getDomHook(endpoint, opt_moduleName);
    const moduleName = opt_moduleName || domHook.getModuleName();
-    pluginEndpoints.registerModule(this, {
+    getPluginEndpoints().registerModule(this, {
      slot,
      endpoint,
      type,