Bug 1553799 - Part 3: Move serviceContainer to its own module r=Ola

Differential Revision: https://phabricator.services.mozilla.com/D39374

--HG--
extra : moz-landing-system : lando

devtools/client/application/initializer.js

@@ -34,6 +34,8 @@ const {
   removeDebugServiceWorkersListener,
 } = require("devtools/shared/service-workers-debug-helper");
 
+const { services } = require("./src/modules/services");
+
 const App = createFactory(require("./src/components/App"));
 
 /**
@@ -54,11 +56,7 @@ window.Application = {
     this.actions = bindActionCreators(actions, this.store.dispatch);
     this.serviceWorkerRegistrationFronts = [];
 
-    const serviceContainer = {
-      selectTool(toolId) {
-        return toolbox.selectTool(toolId);
-      },
-    };
+    services.init(this.toolbox);
 
     this.workersListener = new WorkersListener(this.client.mainRoot);
     this.workersListener.addListener(this.updateWorkers);
@@ -76,7 +74,6 @@ window.Application = {
     const app = App({
       client: this.client,
       fluentBundles: l10n.getBundles(),
-      serviceContainer,
     });
     render(Provider({ store: this.store }, app), this.mount);
   },

devtools/client/application/src/components/App.js

@@ -22,24 +22,16 @@ const WorkersPage = createFactory(require("./service-workers/WorkersPage"));
 class App extends PureComponent {
   static get propTypes() {
     return {
-      client: PropTypes.object.isRequired,
       fluentBundles: PropTypes.array.isRequired,
-      serviceContainer: PropTypes.object.isRequired,
     };
   }
 
   render() {
-    const { client, fluentBundles, serviceContainer } = this.props;
+    const { fluentBundles } = this.props;
 
     return LocalizationProvider(
       { messages: fluentBundles },
-      main(
-        { className: `application` },
-        WorkersPage({
-          client,
-          serviceContainer,
-        })
-      )
+      main({ className: `application` }, WorkersPage({}))
     );
   }
 }

devtools/client/application/src/components/service-workers/Worker.js

@@ -33,12 +33,6 @@ const Localized = createFactory(FluentReact.Localized);
 
 const UIButton = createFactory(require("../ui/UIButton"));
 
-loader.lazyRequireGetter(
-  this,
-  "DebuggerClient",
-  "devtools/shared/client/debugger-client",
-  true
-);
 loader.lazyRequireGetter(
   this,
   "gDevToolsBrowser",
@@ -55,7 +49,6 @@ loader.lazyRequireGetter(
 class Worker extends PureComponent {
   static get propTypes() {
     return {
-      client: PropTypes.instanceOf(DebuggerClient).isRequired,
       isDebugEnabled: PropTypes.bool.isRequired,
       worker: PropTypes.shape({
         active: PropTypes.bool,

devtools/client/application/src/components/service-workers/WorkerList.js

@@ -31,13 +31,12 @@ class WorkerList extends PureComponent {
   static get propTypes() {
     return {
       canDebugWorkers: PropTypes.bool.isRequired,
-      client: PropTypes.object.isRequired,
       workers: PropTypes.array.isRequired,
     };
   }
 
   render() {
-    const { canDebugWorkers, client, workers } = this.props;
+    const { canDebugWorkers, workers } = this.props;
 
     return [
       article(
@@ -53,7 +52,6 @@ class WorkerList extends PureComponent {
           workers.map(worker =>
             Worker({
               key: worker.id,
-              client,
               isDebugEnabled: canDebugWorkers,
               worker,
             })
@@ -76,5 +74,4 @@ class WorkerList extends PureComponent {
 }
 
 // Exports
-
 module.exports = WorkerList;

devtools/client/application/src/components/service-workers/WorkerListEmpty.js

@@ -5,7 +5,6 @@
 "use strict";
 
 const { openDocLink, openTrustedLink } = require("devtools/client/shared/link");
-const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const {
   createFactory,
   PureComponent,
@@ -21,6 +20,8 @@ const {
 const FluentReact = require("devtools/client/shared/vendor/fluent-react");
 const Localized = createFactory(FluentReact.Localized);
 
+const { services } = require("../../modules/services");
+
 const DOC_URL =
   "https://developer.mozilla.org/docs/Web/API/Service_Worker_API/Using_Service_Workers" +
   "?utm_source=devtools&utm_medium=sw-panel-blank";
@@ -30,18 +31,12 @@ const DOC_URL =
  * current target.
  */
 class WorkerListEmpty extends PureComponent {
-  static get propTypes() {
-    return {
-      serviceContainer: PropTypes.object.isRequired,
-    };
-  }
-
   switchToConsole() {
-    this.props.serviceContainer.selectTool("webconsole");
+    services.selectTool("webconsole");
   }
 
   switchToDebugger() {
-    this.props.serviceContainer.selectTool("jsdebugger");
+    services.selectTool("jsdebugger");
   }
 
   openAboutDebugging() {
@@ -98,5 +93,4 @@ class WorkerListEmpty extends PureComponent {
 }
 
 // Exports
-
 module.exports = WorkerListEmpty;

devtools/client/application/src/components/service-workers/WorkersPage.js

@@ -20,8 +20,6 @@ const WorkerListEmpty = createFactory(require("./WorkerListEmpty"));
 class WorkersPage extends PureComponent {
   static get propTypes() {
     return {
-      client: PropTypes.object.isRequired,
-      serviceContainer: PropTypes.object.isRequired,
       // mapped from state
       canDebugWorkers: PropTypes.bool.isRequired,
       domain: PropTypes.string.isRequired,
@@ -30,13 +28,7 @@ class WorkersPage extends PureComponent {
   }
 
   render() {
-    const {
-      canDebugWorkers,
-      client,
-      domain,
-      serviceContainer,
-      workers,
-    } = this.props;
+    const { canDebugWorkers, domain, workers } = this.props;
 
     // Filter out workers from other domains
     const domainWorkers = workers.filter(
@@ -51,10 +43,9 @@ class WorkersPage extends PureComponent {
         }`,
       },
       isWorkerListEmpty
-        ? WorkerListEmpty({ serviceContainer })
+        ? WorkerListEmpty({})
         : WorkerList({
             canDebugWorkers,
-            client,
             workers: domainWorkers,
           })
     );
@@ -70,5 +61,4 @@ function mapStateToProps(state) {
 }
 
 // Exports
-
 module.exports = connect(mapStateToProps)(WorkersPage);

devtools/client/application/src/modules/moz.build

@@ -4,4 +4,5 @@
 
 DevToolsModules(
     'l10n.js',
+    'services.js',
 )

devtools/client/application/src/modules/services.js

@@ -0,0 +1,21 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+class Services {
+  init(toolbox) {
+    this._toolbox = toolbox;
+  }
+
+  selectTool(toolId) {
+    if (!this._toolbox) {
+      throw new Error("Services singleton has not been initialized");
+    }
+    return this._toolbox.selectTool(toolId);
+  }
+}
+
+// exports a singleton, which will be used across all application panel modules
+exports.services = new Services();

devtools/client/application/test/components/service-workers/__snapshots__/components_application_panel-WorkerList.test.js.snap

@@ -15,7 +15,6 @@ Array [
     </Localized>
     <ul>
       <Worker
-        client={Object {}}
         isDebugEnabled={true}
         worker={
           Object {
@@ -62,7 +61,6 @@ Array [
     </Localized>
     <ul>
       <Worker
-        client={Object {}}
         isDebugEnabled={true}
         worker={
           Object {
@@ -76,7 +74,6 @@ Array [
         }
       />
       <Worker
-        client={Object {}}
         isDebugEnabled={true}
         worker={
           Object {
@@ -90,7 +87,6 @@ Array [
         }
       />
       <Worker
-        client={Object {}}
         isDebugEnabled={true}
         worker={
           Object {

devtools/client/application/test/components/service-workers/__snapshots__/components_application_panel-WorkersPage.test.js.snap

@@ -6,7 +6,6 @@ exports[`WorkersPage filters out workers from diferent domains 1`] = `
 >
   <WorkerList
     canDebugWorkers={true}
-    client={Object {}}
     workers={
       Array [
         Object {
@@ -31,33 +30,42 @@ exports[`WorkersPage filters out workers from diferent domains 1`] = `
 </section>
 `;
 
-exports[`WorkersPage filters out workers from different domains and renders an empty list if there is none left 1`] = `
+exports[`WorkersPage filters out workers from different domains and renders an empty list when there is none left 1`] = `
 <section
   className="workers-page workers-page--empty"
 >
-  <WorkerListEmpty
-    serviceContainer={Object {}}
-  />
+  <WorkerListEmpty />
 </section>
 `;
 
-exports[`WorkersPage renders the expected snapshot for a WorkerPage with empty worker list 1`] = `
-<section
-  className="workers-page workers-page--empty"
->
-  <WorkerListEmpty
-    serviceContainer={Object {}}
-  />
-</section>
-`;
-
-exports[`WorkersPage renders the expected snapshot for an App with multiple workers list 1`] = `
+exports[`WorkersPage it renders a list with a single element if there's just 1 worker 1`] = `
+<section
+  className="workers-page "
+>
+  <WorkerList
+    canDebugWorkers={true}
+    workers={
+      Array [
+        Object {
+          "active": true,
+          "name": "worker1",
+          "registrationFront": "",
+          "scope": "SCOPE 123",
+          "url": "http://example.com/worker.js",
+          "workerTargetFront": "",
+        },
+      ]
+    }
+  />
+</section>
+`;
+
+exports[`WorkersPage renders a list with multiple elements when there are multiple workers 1`] = `
 <section
   className="workers-page "
 >
   <WorkerList
     canDebugWorkers={true}
-    client={Object {}}
     workers={
       Array [
         Object {
@@ -90,25 +98,10 @@ exports[`WorkersPage renders the expected snapshot for an App with multiple work
 </section>
 `;
 
-exports[`WorkersPage renders the expected snapshot for an App with single worker list 1`] = `
+exports[`WorkersPage renders an empty list if there are no workers 1`] = `
 <section
-  className="workers-page "
+  className="workers-page workers-page--empty"
 >
-  <WorkerList
-    canDebugWorkers={true}
-    client={Object {}}
-    workers={
-      Array [
-        Object {
-          "active": true,
-          "name": "worker1",
-          "registrationFront": "",
-          "scope": "SCOPE 123",
-          "url": "http://example.com/worker.js",
-          "workerTargetFront": "",
-        },
-      ]
-    }
-  />
+  <WorkerListEmpty />
 </section>
 `;

devtools/client/application/test/components/service-workers/components_application_panel-WorkerList.test.js

@@ -5,7 +5,7 @@
 
 // Import libs
 const { shallow } = require("enzyme");
-const React = require("react");
+const { createFactory } = require("react");
 
 // Import constants
 const {
@@ -13,7 +13,7 @@ const {
   MULTIPLE_WORKER_LIST,
 } = require("devtools/client/application/test/components/fixtures/data/constants");
 
-const WorkerList = React.createFactory(
+const WorkerList = createFactory(
   require("devtools/client/application/src/components/service-workers/WorkerList")
 );
 
@@ -24,7 +24,6 @@ describe("WorkerList", () => {
   it("renders the expected snapshot for a list with a single worker", () => {
     const wrapper = shallow(
       WorkerList({
-        client: {},
         workers: SINGLE_WORKER_DEFAULT_DOMAIN_LIST,
         canDebugWorkers: true,
       })
@@ -35,7 +34,6 @@ describe("WorkerList", () => {
   it("renders the expected snapshot for a multiple workers list", () => {
     const wrapper = shallow(
       WorkerList({
-        client: {},
         workers: MULTIPLE_WORKER_LIST,
         canDebugWorkers: true,
       })

devtools/client/application/test/components/service-workers/components_application_panel-WorkerListEmpty.test.js

@@ -5,9 +5,9 @@
 
 // Import libs
 const { shallow } = require("enzyme");
-const React = require("react");
+const { createFactory } = require("react");
 
-const WorkerListEmpty = React.createFactory(
+const WorkerListEmpty = createFactory(
   require("devtools/client/application/src/components/service-workers/WorkerListEmpty")
 );
 
@@ -17,11 +17,7 @@ const WorkerListEmpty = React.createFactory(
 
 describe("WorkerListEmpty", () => {
   it("renders the expected snapshot", () => {
-    const wrapper = shallow(
-      WorkerListEmpty({
-        serviceContainer: {},
-      })
-    );
+    const wrapper = shallow(WorkerListEmpty({}));
     expect(wrapper).toMatchSnapshot();
   });
 });

devtools/client/application/test/components/service-workers/components_application_panel-WorkersPage.test.js

@@ -5,7 +5,7 @@
 
 // Import libs
 const { shallow } = require("enzyme");
-const React = require("react");
+const { createFactory } = require("react");
 
 // Import fixtures
 const {
@@ -22,7 +22,7 @@ const {
 } = require("devtools/client/application/test/components/helpers/helpers");
 
 // Import component
-const WorkersPage = React.createFactory(
+const WorkersPage = createFactory(
   require("devtools/client/application/src/components/service-workers/WorkersPage")
 );
 
@@ -43,52 +43,28 @@ describe("WorkersPage", () => {
 
   it("renders an empty list if there are no workers", () => {
     const store = buildStoreWithWorkers(EMPTY_WORKER_LIST);
-
-    const wrapper = shallow(
-      WorkersPage({ store, client: {}, serviceContainer: {} })
-    ).dive();
+    const wrapper = shallow(WorkersPage({ store })).dive();
 
     expect(wrapper).toMatchSnapshot();
   });
 
   it("it renders a list with a single element if there's just 1 worker", () => {
     const store = buildStoreWithWorkers(SINGLE_WORKER_DEFAULT_DOMAIN_LIST);
-
-    const wrapper = shallow(
-      WorkersPage({
-        store,
-        client: {},
-        serviceContainer: {},
-      })
-    ).dive();
+    const wrapper = shallow(WorkersPage({ store })).dive();
 
     expect(wrapper).toMatchSnapshot();
   });
 
   it("renders a list with multiple elements when there are multiple workers", () => {
     const store = buildStoreWithWorkers(MULTIPLE_WORKER_LIST);
-
-    const wrapper = shallow(
-      WorkersPage({
-        store,
-        client: {},
-        serviceContainer: {},
-      })
-    ).dive();
+    const wrapper = shallow(WorkersPage({ store })).dive();
 
     expect(wrapper).toMatchSnapshot();
   });
 
   it("filters out workers from diferent domains", () => {
     const store = buildStoreWithWorkers(MULTIPLE_WORKER_MIXED_DOMAINS_LIST);
-
-    const wrapper = shallow(
-      WorkersPage({
-        store,
-        client: {},
-        serviceContainer: {},
-      })
-    ).dive();
+    const wrapper = shallow(WorkersPage({ store })).dive();
 
     expect(wrapper).toMatchSnapshot();
   });
@@ -98,14 +74,7 @@ describe("WorkersPage", () => {
       "there is none left",
     () => {
       const store = buildStoreWithWorkers(SINGLE_WORKER_DIFFERENT_DOMAIN_LIST);
-
-      const wrapper = shallow(
-        WorkersPage({
-          store,
-          client: {},
-          serviceContainer: {},
-        })
-      ).dive();
+      const wrapper = shallow(WorkersPage({ store })).dive();
 
       expect(wrapper).toMatchSnapshot();
     }