moved on start to request class and fixed event ordering

Author: joetannenbaum

packages/core/src/request.ts

@@ -1,5 +1,5 @@
 import { default as axios, AxiosProgressEvent, AxiosRequestConfig, AxiosResponse } from 'axios'
-import { fireExceptionEvent, fireFinishEvent, fireProgressEvent } from './events'
+import { fireExceptionEvent, fireFinishEvent, fireProgressEvent, fireStartEvent } from './events'
 import { page as currentPage } from './page'
 import { RequestParams } from './requestParams'
 import { Response } from './response'
@@ -17,14 +17,18 @@ export class Request {
   ) {
     this.requestParams = RequestParams.create(params)
     this.cancelToken = new AbortController()
-    this.requestParams.onCancelToken(() => this.cancel({ cancelled: true }))
   }
 
   public static create(params: ActiveVisit, page: Page): Request {
     return new Request(params, page)
   }
 
   public async send() {
+    this.requestParams.onCancelToken(() => this.cancel({ cancelled: true }))
+
+    fireStartEvent(this.requestParams.all())
+    this.requestParams.onStart()
+
     return axios({
       method: this.requestParams.params.method,
       url: urlWithoutHash(this.requestParams.params.url).href,

packages/core/src/requestParams.ts

@@ -51,6 +51,10 @@ export class RequestParams {
     this.params.onFinish(this.params)
   }
 
+  public onStart() {
+    this.params.onStart(this.params)
+  }
+
   public all() {
     return this.params
   }

packages/core/src/router.ts

@@ -1,5 +1,5 @@
 import debounce from './debounce'
-import { fireBeforeEvent, fireNavigateEvent, fireStartEvent } from './events'
+import { fireBeforeEvent, fireNavigateEvent } from './events'
 import { hasFiles } from './files'
 import { isFormData, objectToFormData } from './formData'
 import { History } from './history'
@@ -169,9 +169,6 @@ export class Router {
     // Save scroll regions for the current page
     Scroll.save(currentPage.get())
 
-    fireStartEvent(visit)
-    onStart(visit)
-
     const request = Request.create(
       {
         ...visit,

packages/core/tests/request.test.ts

@@ -152,14 +152,17 @@ test.each([
 
   const abortSpy = vi.spyOn(AbortController.prototype, 'abort')
   const fireFinishEventsSpy = vi.spyOn(events, 'fireFinishEvent').mockReturnValue()
+  const fireStartEventsSpy = vi.spyOn(events, 'fireStartEvent').mockReturnValue()
   const onCancel = vi.fn()
   const onFinish = vi.fn()
+  const onStart = vi.fn()
 
   axiosMock.mockResolvedValue(axiosResponse())
 
   const requestParams = getRequestParams({
     onCancel,
     onFinish,
+    onStart,
   })
 
   const request = Request.create(requestParams, page.get())
@@ -180,6 +183,12 @@ test.each([
   expect(fireFinishEventsSpy).toHaveBeenCalledOnce()
   expect(fireFinishEventsSpy).toHaveBeenCalledWith(finalParams)
 
+  expect(fireStartEventsSpy).toHaveBeenCalledOnce()
+  expect(fireStartEventsSpy).toHaveBeenCalledWith(finalParams)
+
+  expect(onStart).toHaveBeenCalledOnce()
+  expect(onStart).toHaveBeenCalledWith(finalParams)
+
   expect(onFinish).toHaveBeenCalledOnce()
   expect(onFinish).toHaveBeenCalledWith(finalParams)
 

packages/core/tests/router.test.ts

@@ -415,34 +415,6 @@ suite('visit', () => {
     expect(requestSpies.send).toHaveBeenCalledOnce()
   })
 
-  test('start event callbacks are fired', async () => {
-    const requestSpies = {
-      create: vi.spyOn(Request, 'create'),
-      send: vi.spyOn(Request.prototype, 'send').mockResolvedValue(),
-    }
-
-    const globalOnStart = vi.fn()
-    const localOnStart = vi.fn()
-
-    const router = getRouter()
-
-    const stopListening = router.on('start', globalOnStart)
-
-    router.visit('/home', {
-      onStart: localOnStart,
-    })
-
-    await vi.runAllTimersAsync()
-
-    stopListening()
-
-    expect(requestSpies.create).toHaveBeenCalledOnce()
-    expect(requestSpies.send).toHaveBeenCalledOnce()
-
-    expect(globalOnStart).toHaveBeenCalledOnce()
-    expect(localOnStart).toHaveBeenCalledOnce()
-  })
-
   test('verify that request params are passed to the request', { todo: true }, async () => {
     const requestSpies = {
       create: vi.spyOn(Request, 'create'),