feat(GUI): remove angular from image-writer (#1680)

* feat(GUI): remove angular from image-writer

We remove Angular from the `image-writer` module by using Redux store
updates, subscribing to them while flashing.

Changelog-Entry: Remove Angular dependency from image-writer.
Change-Type: minor

lib/gui/modules/image-writer.js

@@ -16,11 +16,7 @@
 
 'use strict'
 
-/**
- * @module Etcher.Modules.ImageWriter
- */
-
-const angular = require('angular')
+const Bluebird = require('bluebird')
 const _ = require('lodash')
 const childWriter = require('../../child-writer')
 const settings = require('../models/settings')
@@ -29,118 +25,105 @@ const errors = require('../../shared/errors')
 const windowProgress = require('../os/window-progress')
 const analytics = require('../modules/analytics')
 
-const MODULE_NAME = 'Etcher.Modules.ImageWriter'
-const imageWriter = angular.module(MODULE_NAME, [])
-
-imageWriter.service('ImageWriterService', function ($q, $rootScope) {
-  /**
-   * @summary Perform write operation
-   * @function
-   * @private
-   *
-   * @description
-   * This function is extracted for testing purposes.
-   *
-   * @param {String} image - image path
-   * @param {Object} drive - drive
-   * @param {Function} onProgress - in progress callback (state)
-   *
-   * @fulfil {Object} - flash results
-   * @returns {Promise}
-   *
-   * @example
-   * ImageWriter.performWrite('path/to/image.img', {
-   *   device: '/dev/disk2'
-   * }, (state) => {
-   *   console.log(state.percentage);
-   * });
-   */
-  this.performWrite = (image, drive, onProgress) => {
-    return $q((resolve, reject) => {
-      const child = childWriter.write(image, drive, {
-        validateWriteOnSuccess: settings.get('validateWriteOnSuccess'),
-        unmountOnSuccess: settings.get('unmountOnSuccess')
-      })
-      child.on('error', reject)
-      child.on('done', resolve)
-      child.on('progress', onProgress)
+/**
+ * @summary Perform write operation
+ * @function
+ * @private
+ *
+ * @description
+ * This function is extracted for testing purposes.
+ *
+ * @param {String} image - image path
+ * @param {Object} drive - drive
+ * @param {Function} onProgress - in progress callback (state)
+ *
+ * @fulfil {Object} - flash results
+ * @returns {Promise}
+ *
+ * @example
+ * imageWriter.performWrite('path/to/image.img', {
+ *   device: '/dev/disk2'
+ * }, (state) => {
+ *   console.log(state.percentage)
+ * })
+ */
+exports.performWrite = (image, drive, onProgress) => {
+  return new Bluebird((resolve, reject) => {
+    const child = childWriter.write(image, drive, {
+      validateWriteOnSuccess: settings.get('validateWriteOnSuccess'),
+      unmountOnSuccess: settings.get('unmountOnSuccess')
     })
+    child.on('error', reject)
+    child.on('done', resolve)
+    child.on('progress', onProgress)
+  })
+}
+
+/**
+ * @summary Flash an image to a drive
+ * @function
+ * @public
+ *
+ * @description
+ * This function will update `imageWriter.state` with the current writing state.
+ *
+ * @param {String} image - image path
+ * @param {Object} drive - drive
+ * @returns {Promise}
+ *
+ * @example
+ * imageWriter.flash('foo.img', {
+ *   device: '/dev/disk2'
+ * }).then(() => {
+ *   console.log('Write completed!')
+ * })
+ */
+exports.flash = (image, drive) => {
+  if (flashState.isFlashing()) {
+    return Bluebird.reject(new Error('There is already a flash in progress'))
   }
 
-  /**
-   * @summary Flash an image to a drive
-   * @function
-   * @public
-   *
-   * @description
-   * This function will update `ImageWriterService.state` with the current writing state.
-   *
-   * @param {String} image - image path
-   * @param {Object} drive - drive
-   * @returns {Promise}
-   *
-   * @example
-   * ImageWriterService.flash('foo.img', {
-   *   device: '/dev/disk2'
-   * }).then(() => {
-   *   console.log('Write completed!');
-   * });
-   */
-  this.flash = (image, drive) => {
-    if (flashState.isFlashing()) {
-      return $q.reject(new Error('There is already a flash in progress'))
-    }
+  flashState.setFlashingFlag()
 
-    flashState.setFlashingFlag()
-
-    const analyticsData = {
-      image,
-      drive,
-      uuid: flashState.getFlashUuid(),
-      unmountOnSuccess: settings.get('unmountOnSuccess'),
-      validateWriteOnSuccess: settings.get('validateWriteOnSuccess')
-    }
-
-    analytics.logEvent('Flash', analyticsData)
-
-    return this.performWrite(image, drive, (state) => {
-      // Bring this value to the world of angular.
-      // If we don't trigger a digest loop,
-      // `.getFlashState()` will not return
-      // the latest updated progress state.
-      $rootScope.$apply(() => {
-        flashState.setProgressState(state)
-      })
-    }).then(flashState.unsetFlashingFlag).then(() => {
-      if (flashState.wasLastFlashCancelled()) {
-        analytics.logEvent('Elevation cancelled', analyticsData)
-      } else {
-        analytics.logEvent('Done', analyticsData)
-      }
-    }).catch((error) => {
-      flashState.unsetFlashingFlag({
-        errorCode: error.code
-      })
-
-      if (error.code === 'EVALIDATION') {
-        analytics.logEvent('Validation error', analyticsData)
-      } else if (error.code === 'EUNPLUGGED') {
-        analytics.logEvent('Drive unplugged', analyticsData)
-      } else if (error.code === 'EIO') {
-        analytics.logEvent('Input/output error', analyticsData)
-      } else if (error.code === 'ENOSPC') {
-        analytics.logEvent('Out of space', analyticsData)
-      } else {
-        analytics.logEvent('Flash error', _.merge({
-          error: errors.toJSON(error)
-        }, analyticsData))
-      }
-
-      return $q.reject(error)
-    }).finally(() => {
-      windowProgress.clear()
-    })
+  const analyticsData = {
+    image,
+    drive,
+    uuid: flashState.getFlashUuid(),
+    unmountOnSuccess: settings.get('unmountOnSuccess'),
+    validateWriteOnSuccess: settings.get('validateWriteOnSuccess')
   }
-})
 
-module.exports = MODULE_NAME
+  analytics.logEvent('Flash', analyticsData)
+
+  return exports.performWrite(image, drive, (state) => {
+    flashState.setProgressState(state)
+  }).then(flashState.unsetFlashingFlag).then(() => {
+    if (flashState.wasLastFlashCancelled()) {
+      analytics.logEvent('Elevation cancelled', analyticsData)
+    } else {
+      analytics.logEvent('Done', analyticsData)
+    }
+  }).catch((error) => {
+    flashState.unsetFlashingFlag({
+      errorCode: error.code
+    })
+
+    if (error.code === 'EVALIDATION') {
+      analytics.logEvent('Validation error', analyticsData)
+    } else if (error.code === 'EUNPLUGGED') {
+      analytics.logEvent('Drive unplugged', analyticsData)
+    } else if (error.code === 'EIO') {
+      analytics.logEvent('Input/output error', analyticsData)
+    } else if (error.code === 'ENOSPC') {
+      analytics.logEvent('Out of space', analyticsData)
+    } else {
+      analytics.logEvent('Flash error', _.merge({
+        error: errors.toJSON(error)
+      }, analyticsData))
+    }
+
+    return Bluebird.reject(error)
+  }).finally(() => {
+    windowProgress.clear()
+  })
+}

lib/gui/pages/main/controllers/flash.js

@@ -22,11 +22,13 @@ const driveScanner = require('../../../modules/drive-scanner')
 const progressStatus = require('../../../modules/progress-status')
 const notification = require('../../../os/notification')
 const exceptionReporter = require('../../../modules/exception-reporter')
+const imageWriter = require('../../../modules/image-writer')
 const path = require('path')
+const store = require('../../../../shared/store')
 
 module.exports = function (
   $state,
-  ImageWriterService,
+  $timeout,
   FlashErrorModalService
 ) {
   /**
@@ -53,7 +55,7 @@ module.exports = function (
    *   size: 99999,
    *   mountpoint: '/mnt/foo',
    *   system: false
-   * });
+   * })
    */
   this.flashImageToDrive = (image, drive) => {
     if (flashState.isFlashing()) {
@@ -64,9 +66,13 @@ module.exports = function (
     // otherwise Windows throws EPERM
     driveScanner.stop()
 
+    // Trigger Angular digests along with store updates, as the flash state
+    // updates. Without this there is essentially no progress to watch.
+    const unsubscribe = store.subscribe($timeout)
+
     const iconPath = '../../assets/icon.png'
 
-    ImageWriterService.flash(image.path, drive).then(() => {
+    imageWriter.flash(image.path, drive).then(() => {
       if (!flashState.wasLastFlashCancelled()) {
         notification.send('Success!', {
           body: messages.info.flashComplete({
@@ -102,8 +108,10 @@ module.exports = function (
         FlashErrorModalService.show(messages.error.genericFlashError())
         exceptionReporter.report(error)
       }
-    }).finally(() => {
+    })
+    .finally(() => {
       driveScanner.start()
+      unsubscribe()
     })
   }
 
@@ -115,7 +123,7 @@ module.exports = function (
    * @returns {String} progress button label
    *
    * @example
-   * const label = FlashController.getProgressButtonLabel();
+   * const label = FlashController.getProgressButtonLabel()
    */
   this.getProgressButtonLabel = () => {
     if (!flashState.isFlashing()) {

lib/gui/pages/main/main.js

@@ -42,8 +42,6 @@ const MainPage = angular.module(MODULE_NAME, [
   require('../../os/open-external/open-external'),
   require('../../os/dropzone/dropzone'),
 
-  require('../../modules/image-writer'),
-
   require('../../utils/byte-size/byte-size')
 ])
 

tests/gui/modules/image-writer.spec.js

@@ -1,132 +1,99 @@
-/*
- * Copyright 2017 resin.io
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
 'use strict'
 
 const m = require('mochainon')
 const angular = require('angular')
+const Bluebird = require('bluebird')
 const flashState = require('../../../lib/shared/models/flash-state')
+const imageWriter = require('../../../lib/gui/modules/image-writer')
 require('angular-mocks')
 
-describe('Browser: ImageWriter', function () {
-  beforeEach(angular.mock.module(
-    require('../../../lib/gui/modules/image-writer')
-  ))
+describe('Browser: imageWriter', () => {
+  describe('.flash()', () => {
+    describe('given a successful write', () => {
+      beforeEach(() => {
+        this.performWriteStub = m.sinon.stub(imageWriter, 'performWrite')
+        this.performWriteStub.returns(Bluebird.resolve({
+          cancelled: false,
+          sourceChecksum: '1234'
+        }))
+      })
 
-  describe('ImageWriterService', function () {
-    let $q
-    let $rootScope
-    let ImageWriterService
+      afterEach(() => {
+        this.performWriteStub.restore()
+      })
 
-    beforeEach(angular.mock.inject(function (_$q_, _$rootScope_, _ImageWriterService_) {
-      $q = _$q_
-      $rootScope = _$rootScope_
-      ImageWriterService = _ImageWriterService_
-    }))
-
-    describe('.flash()', function () {
-      describe('given a successful write', function () {
-        beforeEach(function () {
-          this.performWriteStub = m.sinon.stub(ImageWriterService, 'performWrite')
-          this.performWriteStub.returns($q.resolve({
-            cancelled: false,
-            sourceChecksum: '1234'
-          }))
+      it('should set flashing to false when done', () => {
+        flashState.unsetFlashingFlag({
+          cancelled: false,
+          sourceChecksum: '1234'
         })
 
-        afterEach(function () {
-          this.performWriteStub.restore()
-        })
-
-        it('should set flashing to false when done', function () {
-          flashState.unsetFlashingFlag({
-            cancelled: false,
-            sourceChecksum: '1234'
-          })
-
-          ImageWriterService.flash('foo.img', '/dev/disk2')
-          $rootScope.$apply()
+        imageWriter.flash('foo.img', '/dev/disk2').finally(() => {
           m.chai.expect(flashState.isFlashing()).to.be.false
         })
+      })
 
-        it('should prevent writing more than once', function () {
-          flashState.unsetFlashingFlag({
-            cancelled: false,
-            sourceChecksum: '1234'
-          })
-
-          ImageWriterService.flash('foo.img', '/dev/disk2')
-          ImageWriterService.flash('foo.img', '/dev/disk2').catch(angular.noop)
-          $rootScope.$apply()
-          m.chai.expect(this.performWriteStub).to.have.been.calledOnce
+      it('should prevent writing more than once', () => {
+        flashState.unsetFlashingFlag({
+          cancelled: false,
+          sourceChecksum: '1234'
         })
 
-        it('should reject the second flash attempt', function () {
-          ImageWriterService.flash('foo.img', '/dev/disk2')
+        const writing = imageWriter.flash('foo.img', '/dev/disk2')
+        imageWriter.flash('foo.img', '/dev/disk2').catch(angular.noop)
+        writing.finally(() => {
+          m.chai.expect(this.performWriteStub).to.have.been.calledOnce
+        })
+      })
 
-          let rejectError = null
-          ImageWriterService.flash('foo.img', '/dev/disk2').catch(function (error) {
-            rejectError = error
-          })
-
-          $rootScope.$apply()
+      it('should reject the second flash attempt', () => {
+        imageWriter.flash('foo.img', '/dev/disk2')
 
+        let rejectError = null
+        imageWriter.flash('foo.img', '/dev/disk2').catch((error) => {
+          rejectError = error
+        }).finally(() => {
           m.chai.expect(rejectError).to.be.an.instanceof(Error)
           m.chai.expect(rejectError.message).to.equal('There is already a flash in progress')
         })
       })
+    })
 
-      describe('given an unsuccessful write', function () {
-        beforeEach(function () {
-          this.performWriteStub = m.sinon.stub(ImageWriterService, 'performWrite')
-          this.error = new Error('write error')
-          this.error.code = 'FOO'
-          this.performWriteStub.returns($q.reject(this.error))
-        })
+    describe('given an unsuccessful write', () => {
+      beforeEach(() => {
+        this.performWriteStub = m.sinon.stub(imageWriter, 'performWrite')
+        this.error = new Error('write error')
+        this.error.code = 'FOO'
+        this.performWriteStub.returns(Bluebird.reject(this.error))
+      })
 
-        afterEach(function () {
-          this.performWriteStub.restore()
-        })
+      afterEach(() => {
+        this.performWriteStub.restore()
+      })
 
-        it('should set flashing to false when done', function () {
-          ImageWriterService.flash('foo.img', '/dev/disk2').catch(angular.noop)
-          $rootScope.$apply()
+      it('should set flashing to false when done', () => {
+        imageWriter.flash('foo.img', '/dev/disk2').catch(angular.noop).finally(() => {
           m.chai.expect(flashState.isFlashing()).to.be.false
         })
+      })
 
-        it('should set the error code in the flash results', function () {
-          ImageWriterService.flash('foo.img', '/dev/disk2').catch(angular.noop)
-          $rootScope.$apply()
+      it('should set the error code in the flash results', () => {
+        imageWriter.flash('foo.img', '/dev/disk2').catch(angular.noop).finally(() => {
           const flashResults = flashState.getFlashResults()
           m.chai.expect(flashResults.errorCode).to.equal('FOO')
         })
+      })
 
-        it('should be rejected with the error', function () {
-          flashState.unsetFlashingFlag({
-            cancelled: false,
-            sourceChecksum: '1234'
-          })
-
-          let rejection
-          ImageWriterService.flash('foo.img', '/dev/disk2').catch(function (error) {
-            rejection = error
-          })
-
-          $rootScope.$apply()
+      it('should be rejected with the error', () => {
+        flashState.unsetFlashingFlag({
+          cancelled: false,
+          sourceChecksum: '1234'
+        })
 
+        let rejection
+        imageWriter.flash('foo.img', '/dev/disk2').catch((error) => {
+          rejection = error
+        }).finally(() => {
           m.chai.expect(rejection).to.be.an.instanceof(Error)
           m.chai.expect(rejection.message).to.equal('write error')
         })