fix(setupWorker): warn on multiple "worker.start" and "worker.stop" c…

…alls (#1238)

* fix(setupWorker): print warning on multiple "worker.start()" calls

* fix(setupWorker): print warning on multiple "worker.stop()" calls

src/setupWorker/glossary.ts

@@ -85,6 +85,7 @@ export type ServiceWorkerFetchEventTypes =
 export type WorkerLifecycleEventsMap = LifeCycleEventsMap<Response>
 
 export interface SetupWorkerInternalContext {
+  isMockingEnabled: boolean
   startOptions?: RequiredDeep<StartOptions>
   worker: ServiceWorker | null
   registration: ServiceWorkerRegistration | null

src/setupWorker/setupWorker.ts

@@ -61,6 +61,9 @@ export function setupWorker(
   pipeEvents(emitter, publicEmitter)
 
   const context: SetupWorkerInternalContext = {
+    // Mocking is not considered enabled until the worker
+    // signals back the successful activation event.
+    isMockingEnabled: false,
     startOptions: undefined,
     worker: null,
     registration: null,

src/setupWorker/start/utils/enableMocking.ts

@@ -1,3 +1,4 @@
+import { devUtils } from '../../../utils/internal/devUtils'
 import { StartOptions, SetupWorkerInternalContext } from '../../glossary'
 import { printStartMessage } from './printStartMessage'
 
@@ -9,11 +10,23 @@ export async function enableMocking(
   options: StartOptions,
 ) {
   context.workerChannel.send('MOCK_ACTIVATE')
-  return context.events.once('MOCKING_ENABLED').then(() => {
-    printStartMessage({
-      quiet: options.quiet,
-      workerScope: context.registration?.scope,
-      workerUrl: context.worker?.scriptURL,
-    })
+  await context.events.once('MOCKING_ENABLED')
+
+  // Warn the developer on multiple "worker.start()" calls.
+  // While this will not affect the worker in any way,
+  // it likely indicates an issue with the developer's code.
+  if (context.isMockingEnabled) {
+    devUtils.warn(
+      `Found a redundant "worker.start()" call. Note that starting the worker while mocking is already enabled will have no effect. Consider removing this "worker.start()" call.`,
+    )
+    return
+  }
+
+  context.isMockingEnabled = true
+
+  printStartMessage({
+    quiet: options.quiet,
+    workerScope: context.registration?.scope,
+    workerUrl: context.worker?.scriptURL,
   })
 }

src/setupWorker/stop/createStop.ts

@@ -1,17 +1,29 @@
+import { devUtils } from '../../utils/internal/devUtils'
 import { SetupWorkerInternalContext, StopHandler } from '../glossary'
 import { printStopMessage } from './utils/printStopMessage'
 
 export const createStop = (
   context: SetupWorkerInternalContext,
 ): StopHandler => {
   return function stop() {
+    // Warn developers calling "worker.stop()" more times than necessary.
+    // This likely indicates a mistake in their code.
+    if (!context.isMockingEnabled) {
+      devUtils.warn(
+        'Found a redundant "worker.stop()" call. Note that stopping the worker while mocking already stopped has no effect. Consider removing this "worker.stop()" call.',
+      )
+      return
+    }
+
     /**
      * Signal the Service Worker to disable mocking for this client.
      * Use this an an explicit way to stop the mocking, while preserving
      * the worker-client relation. Does not affect the worker's lifecycle.
      */
     context.workerChannel.send('MOCK_DEACTIVATE')
+    context.isMockingEnabled = false
     window.clearInterval(context.keepAliveInterval)
+
     printStopMessage({ quiet: context.startOptions?.quiet })
   }
 }

test/msw-api/setup-worker/start/start.test.ts

@@ -9,16 +9,19 @@ declare namespace window {
   }
 }
 
-test('resolves the "start" Promise when the worker has been activated', async () => {
-  const runtime = await pageWith({
+function prepareRuntime() {
+  return pageWith({
     example: path.resolve(__dirname, 'start.mocks.ts'),
     routes(app) {
       app.get('/worker.js', (req, res) => {
         res.sendFile(path.resolve(__dirname, 'worker.delayed.js'))
       })
     },
   })
+}
 
+test('resolves the "start" Promise when the worker has been activated', async () => {
+  const runtime = await prepareRuntime()
   const events: string[] = []
 
   const untilWorkerActivated = runtime.page
@@ -59,14 +62,7 @@ test('resolves the "start" Promise when the worker has been activated', async ()
 })
 
 test('prints the start message when the worker has been registered', async () => {
-  const runtime = await pageWith({
-    example: path.resolve(__dirname, 'start.mocks.ts'),
-    routes(app) {
-      app.get('/worker.js', (req, res) => {
-        res.sendFile(path.resolve(__dirname, 'worker.delayed.js'))
-      })
-    },
-  })
+  const runtime = await prepareRuntime()
 
   await runtime.page.evaluate(() => {
     return window.msw.startWorker()
@@ -79,3 +75,21 @@ test('prints the start message when the worker has been registered', async () =>
     `Worker script URL: ${runtime.makeUrl('/worker.js')}`,
   )
 })
+
+test('prints a warning if "worker.start()" is called multiple times', async () => {
+  const runtime = await prepareRuntime()
+
+  await runtime.page.evaluate(() => {
+    return Promise.all([window.msw.startWorker(), window.msw.startWorker()])
+  })
+
+  // The activation message ise printed only once.
+  expect(runtime.consoleSpy.get('startGroupCollapsed')).toEqual([
+    '[MSW] Mocking enabled.',
+  ])
+
+  // The warning is printed about multiple calls of "worker.start()".
+  expect(runtime.consoleSpy.get('warning')).toEqual([
+    `[MSW] Found a redundant "worker.start()" call. Note that starting the worker while mocking is already enabled will have no effect. Consider removing this "worker.start()" call.`,
+  ])
+})

test/msw-api/setup-worker/stop.test.ts

@@ -38,13 +38,14 @@ test('disables the mocking when the worker is stopped', async () => {
 })
 
 test('keeps the mocking enabled in one tab when stopping the worker in another tab', async () => {
-  const { context, origin, server } = await createRuntime()
-  const firstPage = await context.newPage()
-  await firstPage.goto(origin, {
+  const runtime = await createRuntime()
+
+  const firstPage = await runtime.context.newPage()
+  await firstPage.goto(runtime.origin, {
     waitUntil: 'networkidle',
   })
-  const secondPage = await context.newPage()
-  await secondPage.goto(origin, {
+  const secondPage = await runtime.context.newPage()
+  await secondPage.goto(runtime.origin, {
     waitUntil: 'networkidle',
   })
 
@@ -54,7 +55,7 @@ test('keeps the mocking enabled in one tab when stopping the worker in another t
   await secondPage.bringToFront()
 
   // Create a request handler for the new page.
-  const request = createRequestUtil(secondPage, server)
+  const request = createRequestUtil(secondPage, runtime.server)
   const res = await request('https://api.github.com')
   const headers = await res.allHeaders()
   const body = await res.json()
@@ -64,3 +65,27 @@ test('keeps the mocking enabled in one tab when stopping the worker in another t
     mocked: true,
   })
 })
+
+test('prints a warning on multiple "worker.stop()" calls', async () => {
+  const runtime = await createRuntime()
+
+  function byStopMessage(text: string): boolean {
+    return text === '[MSW] Mocking disabled.'
+  }
+
+  await stopWorkerOn(runtime.page)
+
+  // Prints the stop message and no warnings.
+  expect(runtime.consoleSpy.get('log').filter(byStopMessage)).toHaveLength(1)
+  expect(runtime.consoleSpy.get('warning')).toBeUndefined()
+
+  await stopWorkerOn(runtime.page)
+
+  // Does not print a duplicate stop message.
+  expect(runtime.consoleSpy.get('log').filter(byStopMessage)).toHaveLength(1)
+
+  // Prints a warning so the user knows something is not right.
+  expect(runtime.consoleSpy.get('warning')).toEqual([
+    `[MSW] Found a redundant "worker.stop()" call. Note that stopping the worker while mocking already stopped has no effect. Consider removing this "worker.stop()" call.`,
+  ])
+})