fix: fix file watching on compilation error (cypress-io/cypress-webpack-preprocessor#66)

fix issue where compilation error does not cause cypress to refetch the spec and display the error without a manual page reload.

Author: kuceb

.mocharc.json

@@ -0,0 +1,3 @@
+{
+  "watch-ignore": ["./test/_test-output", "node_modules"]
+}

.vscode/settings.json

@@ -1,4 +1,6 @@
 {
   "standard.enable": false,
+  "eslint.enable": true,
+  "eslint.validate": ["json"],
   "git.ignoreLimitWarning": true
-}
+}

circle.yml

@@ -13,17 +13,17 @@ jobs:
 
     steps:
       - checkout
-      
+
       - restore_cache:
           keys:
-          - cache-{{ arch }}-{{ .Branch }}-{{ checksum "package.json" }}
+            - cache-{{ arch }}-{{ .Branch }}-{{ checksum "package.json" }}
       - run:
           name: Yarn install
           command: yarn install --frozen-lockfile
       - save_cache:
           key: cache-{{ arch }}-{{ .Branch }}-{{ checksum "package.json" }}
           paths:
-          - ~/.cache
+            - ~/.cache
       - run:
           name: Lint code
           command: npm run lint
@@ -33,7 +33,10 @@ jobs:
       - run:
           name: Run tests
           command: npm test
-
+      - run: yarn add -D webpack@latest webpack-cli@latest
+      - run:
+          name: Run tests w/ webpack@latest
+          command: npm test
       - run:
           name: Semantic release
           command: npm run semantic-release || true

index.js

@@ -108,14 +108,17 @@ const preprocessor = (options = {}) => {
 
     const rejectWithErr = (err) => {
       err.filePath = filePath
-      debug(`errored bundling ${outputPath}`, err)
+      debug(`errored bundling ${outputPath}`, err.message)
+
       latestBundle.reject(err)
     }
 
     // this function is called when bundling is finished, once at the start
     // and, if watching, each time watching triggers a re-bundle
     const handle = (err, stats) => {
       if (err) {
+        debug('handle - had error', err.message)
+
         return rejectWithErr(err)
       }
 
@@ -126,12 +129,14 @@ const preprocessor = (options = {}) => {
 
         const errorsToAppend = jsonStats.errors
         // remove stack trace lines since they're useless for debugging
-        .map((err) => err.replace(/\n\s*at.*/g, '').replace(/From previous event:\n?/g, ''))
+        .map(cleanseError)
         // multiple errors separated by newline
         .join('\n\n')
 
         err.message += `\n${errorsToAppend}`
 
+        debug('stats had error(s)')
+
         return rejectWithErr(err)
       }
 
@@ -157,7 +162,7 @@ const preprocessor = (options = {}) => {
       latestBundle = createDeferred()
       bundles[filePath] = latestBundle.promise
 
-      bundles[filePath].tap(() => {
+      bundles[filePath].finally(() => {
         debug('- compile finished for', filePath)
         // when the bundling is finished, emit 'rerun' to let Cypress
         // know to rerun the spec
@@ -187,12 +192,12 @@ const preprocessor = (options = {}) => {
 
     // when the spec or project is closed, we need to clean up the cached
     // bundle promise and stop the watcher via `bundler.close()`
-    file.on('close', () => {
+    file.on('close', (cb = function () {}) => {
       debug('close', filePath)
       delete bundles[filePath]
 
       if (file.shouldWatch) {
-        bundler.close()
+        bundler.close(cb)
       }
     })
 
@@ -220,4 +225,8 @@ preprocessor.__reset = () => {
   bundles = {}
 }
 
+function cleanseError (err) {
+  return err.replace(/\n\s*at.*/g, '').replace(/From previous event:\n?/g, '')
+}
+
 module.exports = preprocessor

package.json

@@ -44,14 +44,14 @@
     "fs-extra": "8.1.0",
     "github-post-release": "1.13.1",
     "license-checker": "13.0.3",
-    "mocha": "3.5.0",
+    "mocha": "^7.1.0",
     "mockery": "2.1.0",
     "nsp": "2.7.0",
     "prettier-eslint-cli": "4.4.0",
     "semantic-release": "8.2.0",
     "simple-commit-message": "3.3.1",
-    "sinon": "3.2.1",
-    "sinon-chai": "2.13.0",
+    "sinon": "^9.0.0",
+    "sinon-chai": "^3.5.0",
     "snap-shot-it": "7.9.2",
     "webpack": "^4.18.1"
   },

test/e2e/compilation.spec.js

@@ -1,8 +1,14 @@
 const EventEmitter = require('events').EventEmitter
-const expect = require('chai').expect
+const chai = require('chai')
 const fs = require('fs-extra')
 const path = require('path')
 const snapshot = require('snap-shot-it')
+const sinon = require('sinon')
+const Bluebird = require('bluebird')
+const sinonChai = require('sinon-chai')
+
+chai.use(sinonChai)
+const { expect } = chai
 
 const preprocessor = require('../../index')
 
@@ -12,48 +18,91 @@ describe('webpack preprocessor - e2e', () => {
   let file
   let filePath
 
-  beforeEach(() => {
+  beforeEach(async () => {
     const originalFilePath = path.join(__dirname, '..', 'fixtures', 'example_spec.js')
 
     filePath = path.join(__dirname, '..', '_test-output', 'example_spec.js')
 
     preprocessor.__reset()
-    fs.removeSync(path.join(__dirname, '_test-output'))
-    fs.outputFileSync(filePath, '')
-    fs.copyFileSync(originalFilePath, filePath)
+    await fs.remove(path.join(__dirname, '_test-output'))
+    await fs.outputFile(filePath, '')
+    await fs.copyFile(originalFilePath, filePath)
 
     file = Object.assign(new EventEmitter(), {
       filePath,
       outputPath,
     })
   })
 
+  afterEach(async () => {
+    if (file.shouldWatch) {
+      await new Promise((resolve) => {
+        file.emit('close', resolve)
+      })
+    }
+  })
+
   it('correctly preprocesses the file', () => {
     return preprocessor()(file).then(() => {
       snapshot(fs.readFileSync(outputPath).toString())
     })
   })
 
-  it('allows attaching catch later on syntax error without triggering unhandled rejection', (done) => {
+  it('allows attaching catch later on syntax error without triggering unhandled rejection', async () => {
     process.on('unhandledRejection', (err) => {
       // eslint-disable-next-line no-console
       console.error('Unhandled Rejection:', err.stack)
-      done('Should not have trigger unhandled rejection')
+      throw new Error('Should not have trigger unhandled rejection')
     })
 
     file.shouldWatch = true
 
-    preprocessor()(file).then(() => {
-      fs.outputFileSync(filePath, '{')
+    await preprocessor()(file)
+    await fs.outputFile(filePath, '{')
 
+    await new Promise((resolve) => {
       setTimeout(() => {
         preprocessor()(file)
         .catch((err) => {
           expect(err.stack).to.include('Unexpected token')
-          file.emit('close')
-          done()
+          resolve()
         })
       }, 1000)
     })
   })
+
+  it('triggers rerun on syntax error', async () => {
+    const _emit = sinon.spy(file, 'emit')
+
+    file.shouldWatch = true
+
+    await preprocessor()(file)
+
+    _emit.resetHistory()
+
+    await fs.outputFile(filePath, '{')
+
+    await retry(() => expect(_emit).calledWith('rerun'))
+  })
 })
+
+function retry (fn, timeout = 1000) {
+  let timedOut = false
+
+  setTimeout(() => timedOut = true, timeout)
+  const tryFn = () => {
+    return Bluebird.try(() => {
+      return fn()
+    })
+
+    .catch((err) => {
+      if (timedOut) {
+        throw err
+      }
+
+      return Bluebird.delay(100).then(() => tryFn())
+    })
+  }
+
+  return tryFn()
+}

test/unit/index.spec.js

@@ -8,8 +8,7 @@ const expect = chai.expect
 
 chai.use(require('sinon-chai'))
 
-const sandbox = sinon.sandbox.create()
-const webpack = sandbox.stub()
+const webpack = sinon.stub()
 
 mockery.enable({
   warnOnUnregistered: false,
@@ -22,16 +21,16 @@ const stubbableRequire = require('../../stubbable-require')
 
 describe('webpack preprocessor', function () {
   beforeEach(function () {
-    sandbox.restore()
+    sinon.restore()
 
     this.watchApi = {
-      close: sandbox.spy(),
+      close: sinon.spy(),
     }
 
     this.compilerApi = {
-      run: sandbox.stub(),
-      watch: sandbox.stub().returns(this.watchApi),
-      plugin: sandbox.stub(),
+      run: sinon.stub(),
+      watch: sinon.stub().returns(this.watchApi),
+      plugin: sinon.stub(),
     }
 
     webpack.returns(this.compilerApi)
@@ -49,14 +48,14 @@ describe('webpack preprocessor', function () {
       filePath: 'path/to/file.js',
       outputPath: 'output/output.js',
       shouldWatch: false,
-      on: sandbox.stub(),
-      emit: sandbox.spy(),
+      on: sinon.stub(),
+      emit: sinon.spy(),
     }
 
     this.util = {
-      getOutputPath: sandbox.stub().returns(this.outputPath),
-      fileUpdated: sandbox.spy(),
-      onClose: sandbox.stub(),
+      getOutputPath: sinon.stub().returns(this.outputPath),
+      fileUpdated: sinon.spy(),
+      onClose: sinon.stub(),
     }
 
     this.run = (options, file = this.file) => {
@@ -239,7 +238,7 @@ describe('webpack preprocessor', function () {
       })
 
       it('requires babel dependencies when default options are used', function () {
-        sandbox.spy(stubbableRequire, 'resolve')
+        sinon.spy(stubbableRequire, 'resolve')
 
         return this.run().then(() => {
           expect(stubbableRequire.resolve).to.be.calledWith('babel-loader')
@@ -248,7 +247,7 @@ describe('webpack preprocessor', function () {
       })
 
       it('does not requires babel dependencies when user options are non-default', function () {
-        sandbox.spy(stubbableRequire, 'resolve')
+        sinon.spy(stubbableRequire, 'resolve')
         const options = { webpackOptions: { module: { rules: [] } } }
 
         return this.run(options).then(() => {