feat(npm/webpack-dev-server,runner-ct): Normalize webpack errors + general React/TS improvements (#16613)

* feat: normalize webpack errors

* move some files to TS

* add tests

* simplify test

* improve types

Co-authored-by: Jessica Sachs <[email protected]>

Author: lmiller1990

npm/webpack-dev-server/src/plugin.ts

@@ -33,6 +33,10 @@ export type Webpack45Compilation = Compilation & {
   }
 }
 
+export const normalizeError = (error: Error | string) => {
+  return typeof error === 'string' ? error : error.message
+}
+
 export default class CypressCTOptionsPlugin {
   private files: Cypress.Cypress['spec'][] = []
   private supportFile: string
@@ -64,7 +68,16 @@ export default class CypressCTOptionsPlugin {
 
         if (stats.hasErrors()) {
           this.errorEmitted = true
-          this.devServerEvents.emit('dev-server:compile:error', stats.toJson().errors?.[0])
+
+          // webpack 4: string[]
+          // webpack 5: Error[]
+          const errors = stats.toJson().errors as Array<Error | string> | undefined
+
+          if (!errors || !errors.length) {
+            return
+          }
+
+          this.devServerEvents.emit('dev-server:compile:error', normalizeError(errors[0]))
         } else if (this.errorEmitted) {
           // compilation succeed but assets haven't emitted to the output yet
           this.devServerEvents.emit('dev-server:compile:error', null)

npm/webpack-dev-server/test/e2e.spec.ts

@@ -117,7 +117,11 @@ describe('#startDevServer', () => {
     exitSpy()
 
     return new Promise((res) => {
-      devServerEvents.on('dev-server:compile:error', () => {
+      devServerEvents.on('dev-server:compile:error', (err: string) => {
+        expect(err).to.contain('./test/fixtures/compilation-fails.spec.js 1:5')
+        expect(err).to.contain('Module parse failed: Unexpected token (1:5)')
+        expect(err).to.contain('You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders')
+        expect(err).to.contain('> this is an invalid spec file')
         expect(exitSpy.calledOnce).to.be.true
         close(() => res())
       })

npm/webpack-dev-server/test/unit/makeWebpackConfig.spec.ts

@@ -1,4 +1,5 @@
 import { expect } from 'chai'
+import EventEmitter from 'events'
 import snapshot from 'snap-shot-it'
 import { makeWebpackConfig } from '../../src/makeWebpackConfig'
 
@@ -9,6 +10,7 @@ describe('makeWebpackConfig', () => {
         publicPath: '/this-will-be-ignored',
       },
     }, {
+      devServerEvents: new EventEmitter(),
       devServerPublicPathRoute: '/test-public-path',
       isOpenMode: true,
       supportFile: '/support.js',

npm/webpack-dev-server/test/unit/plugin.spec.ts

@@ -0,0 +1,14 @@
+import { expect } from 'chai'
+import { normalizeError } from '../../src/plugin'
+
+describe('normalizeError', () => {
+  it('normalizes Error to string', () => {
+    const e = new Error('Bad')
+
+    expect(normalizeError(e)).to.eq('Bad')
+  })
+
+  it('returns string as is', () => {
+    expect(normalizeError('Bad')).to.eq('Bad')
+  })
+})

packages/runner-ct/cypress/component/ScriptError.spec.tsx

@@ -0,0 +1,14 @@
+/// <reference types="@percy/cypress" />
+
+import React from 'react'
+import { mount } from '@cypress/react'
+import ScriptError from '../../src/errors/script-error'
+
+describe('ScriptError', () => {
+  it('renders an error', () => {
+    const error = 'There is an error in your code.'
+
+    mount(<ScriptError error={error} />).get('pre').contains(error)
+    cy.percySnapshot()
+  })
+})

packages/runner-ct/src/errors/script-error.jsx

@@ -0,0 +1,28 @@
+import React from 'react'
+import { namedObserver } from '../lib/mobx'
+const ansiToHtml = require('ansi-to-html')
+
+const convert = new ansiToHtml({
+  fg: '#000',
+  bg: '#fff',
+  newline: false,
+  escapeXML: true,
+  stream: false,
+})
+
+export const ScriptError: React.FC<{ error: string }> = namedObserver('ScriptError', ({ error }) => {
+  if (!error) {
+    return null
+  }
+
+  const errorHTML = convert.toHtml(error)
+
+  return (
+    <pre
+      className='script-error'
+      dangerouslySetInnerHTML={{ __html: errorHTML }}
+    />
+  )
+})
+
+export default ScriptError

packages/runner-ct/src/errors/script-error.tsx

@@ -5,10 +5,9 @@ import eventManager from '../lib/event-manager'
 import selectorPlaygroundModel from '../selector-playground/selector-playground-model'
 
 export default class IframeModel {
-  constructor ({ state, detachDom, removeHeadStyles, restoreDom, highlightEl, snapshotControls }) {
+  constructor ({ state, detachDom, restoreDom, highlightEl, snapshotControls }) {
     this.state = state
     this.detachDom = detachDom
-    this.removeHeadStyles = removeHeadStyles
     this.restoreDom = restoreDom
     this.highlightEl = highlightEl
     this.snapshotControls = snapshotControls

packages/runner-ct/src/iframe/iframe-model.js

@@ -2,25 +2,35 @@ import cs from 'classnames'
 import { action, when, autorun } from 'mobx'
 import { observer } from 'mobx-react'
 import React, { Component } from 'react'
-import { $ } from '@packages/driver'
+import { default as $Cypress } from '@packages/driver'
+import State from '../../src/lib/state'
 
 import AutIframe from './aut-iframe'
-import ScriptError from '../errors/script-error'
+import { ScriptError } from '../errors/script-error'
 import SnapshotControls from './snapshot-controls'
 
 import IframeModel from './iframe-model'
 import selectorPlaygroundModel from '../selector-playground/selector-playground-model'
 import styles from '../app/RunnerCt.module.scss'
 import './iframes.scss'
+import eventManager from '../lib/event-manager'
 
 export function getSpecUrl ({ namespace, spec }, prefix = '') {
   return spec ? `${prefix}/${namespace}/iframes/${spec.absolute}` : ''
 }
 
+interface IFramesProps {
+  state: State
+  eventManager: typeof eventManager
+  config: any
+}
+
 @observer
-export default class Iframes extends Component {
+export default class Iframes extends Component<IFramesProps> {
   _disposers = []
   containerRef = null
+  autIframe: AutIframe
+  iframeModel: IframeModel
 
   render () {
     const { viewportHeight, viewportWidth, scriptError, scale, screenshotting } = this.props.state
@@ -84,7 +94,6 @@ export default class Iframes extends Component {
 
     this.iframeModel = new IframeModel({
       state: this.props.state,
-      removeHeadStyles: this.autIframe.removeHeadStyles,
       restoreDom: this.autIframe.restoreDom,
       highlightEl: this.autIframe.highlightEl,
       detachDom: this.autIframe.detachDom,
@@ -110,7 +119,7 @@ export default class Iframes extends Component {
     }))
   }
 
-  @action _setScriptError = (err) => {
+  @action _setScriptError = (err: string | undefined) => {
     this.props.state.scriptError = err
   }
 
@@ -119,7 +128,7 @@ export default class Iframes extends Component {
 
     // this.props.eventManager.notifyRunningSpec(specPath)
     // logger.clearLog()
-    this._setScriptError(null)
+    this._setScriptError(undefined)
 
     this.props.eventManager.setup(config)
 
@@ -137,7 +146,7 @@ export default class Iframes extends Component {
   // wiped out and reset on re-runs and the snapshots are from dom we don't control
   _loadIframes (spec) {
     const specSrc = getSpecUrl({ namespace: this.props.config.namespace, spec })
-    const $container = $(this.containerRef).empty()
+    const $container = $Cypress.$(this.containerRef).empty()
     const $autIframe = this.autIframe.create().appendTo($container)
 
     this.autIframe.showBlankContents()
@@ -147,15 +156,6 @@ export default class Iframes extends Component {
     $autIframe.prop('src', specSrc)
 
     return $autIframe
-
-    // const $specIframe = $('<iframe />', {
-    //   id: `Your Spec: '${specSrc}'`,
-    //   class: 'spec-iframe',
-    // }).appendTo($container)
-
-    // $specIframe.prop('src', specSrc)
-
-    // return $autIframe
   }
 
   _toggleSnapshotHighlights = (snapshotProps) => {
@@ -184,11 +184,7 @@ export default class Iframes extends Component {
   }
 
   componentDidUpdate () {
-    const cb = this.props.state.callbackAfterUpdate
-
-    if (cb) {
-      cb()
-    }
+    this.props.state.callbackAfterUpdate?.()
   }
 
   _printSelectorElementsToConsole = () => {

packages/runner-ct/src/iframe/iframes.jsx renamed to packages/runner-ct/src/iframe/iframes.tsx

@@ -116,7 +116,7 @@ export default class State {
 
   @observable automation = automation.CONNECTING
 
-  @observable.ref scriptError = null
+  @observable.ref scriptError: string | undefined
 
   @observable spec = _defaults.spec
   @observable specs = _defaults.specs

packages/runner-ct/src/lib/state.ts

@@ -10,8 +10,8 @@ export class SocketCt extends SocketBase {
   constructor (config: Record<string, any>) {
     super(config)
 
-    devServer.emitter.on('dev-server:compile:error', (error) => {
-      this.toRunner('dev-server:hmr:error', error === null ? null : { error })
+    devServer.emitter.on('dev-server:compile:error', (error: string | undefined) => {
+      this.toRunner('dev-server:hmr:error', error)
     })
 
     // should we use this option at all for component testing 😕?