Raise on mismatched components instead of warn

This changes `remote` to raise when encountering a page response that has a different component than the target page to save in. We also add a `force: true` option to remote to retain the original behavior.
thoughtbot · Nov 30, 2024 · c2b80b8 · c2b80b8
1 parent 39461a6
commit c2b80b8
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 29 deletions.
diff --git a/docs/requests.md b/docs/requests.md
@@ -65,9 +65,17 @@ sequenceDiagram
   end
 ```
 
-If you provide a `pageKey` you can also target a different page
-in your store not visible to the user.
-
+If you provide a `pageKey` you can also target a different page in your store
+not visible to the user. Unlike `visit`, `remote` will not derive the target
+page key from the response. As long as the componentIdentifier from the
+response and target page is the same, `remote` will save and process the response
+to the provided `pageKey`.
+
+!!! warning
+    The componentIdentifier from the page response **MUST** match the target page, otherwise
+    remote will throw a `MismatchedComponentError` error. You can override this by using the
+    `force: true` option. See the [docs](reference/types.requests.md#remoteprops)
+    for details.
 
 ```mermaid
 sequenceDiagram

diff --git a/superglue/lib/action_creators/requests.ts b/superglue/lib/action_creators/requests.ts
@@ -57,13 +57,33 @@ function buildMeta(
   }
 }
 
+export class MismatchedComponentError extends Error {
+  constructor(existingId: string, receivedId: string, currentPageKey: string) {
+    const message = `You are about to replace an existing page located at pages["${currentPageKey}"]
+that has the componentIdentifier "${existingId}" with the contents of a
+received page that has a componentIdentifier of "${receivedId}".
+
+This can happen if you're using data-sg-remote or remote but your response
+redirected to a completely different page. Since remote requests do not
+navigate or change the current page component, your current page component may
+receive a shape that is unexpected and cause issues with rendering.
+
+Consider using data-sg-visit, the visit function, or redirect_back to the same page. Or if you're
+sure you want to proceed, use force: true.
+`
+    super(message)
+    this.name = 'MismatchedComponentError'
+  }
+}
+
 export const remote: RemoteCreator = (
   path,
   {
     method = 'GET',
     headers,
     body,
     pageKey: rawPageKey,
+    force = false,
     beforeSave = (prevPage: Page, receivedPage: PageResponse) => receivedPage,
   } = {}
 ) => {
@@ -95,18 +115,16 @@ export const remote: RemoteCreator = (
         const existingId = pages[currentPageKey]?.componentIdentifier
         const receivedId = json.componentIdentifier
 
-        if (willReplaceCurrent && !!existingId && existingId != receivedId) {
-          console.warn(
-            `You're about replace an existing page located at pages["${currentPageKey}"]
-that has the componentIdentifier "${existingId}" with the contents of a
-received page that has a componentIdentifier of "${receivedId}".
-
-This can happen if you're using data-sg-remote or remote but your response
-redirected to a completely different page. Since remote requests do not
-navigate or change the current page component, your current page component may
-receive a shape that is unexpected and cause issues with rendering.
-
-Consider using data-sg-visit, the visit function, or redirect_back.`
+        if (
+          willReplaceCurrent &&
+          !!existingId &&
+          existingId != receivedId &&
+          !force
+        ) {
+          throw new MismatchedComponentError(
+            existingId,
+            receivedId,
+            currentPageKey
           )
         }
 
@@ -187,6 +205,7 @@ export const visit: VisitCreator = (
         const isGet = fetchArgs[1].method === 'GET'
 
         meta.suggestedAction = 'push'
+
         if (!rsp.redirected && !isGet) {
           meta.suggestedAction = 'replace'
         }

diff --git a/superglue/lib/types/requests.ts b/superglue/lib/types/requests.ts
@@ -83,6 +83,11 @@ export interface RemoteProps extends BaseProps {
    * {@link Remote} will use the `currentPageKey` at {@link SuperglueState}
    */
   pageKey?: PageKey
+  /**
+   * Forces {@link Remote} to allow mismatched components between the response
+   * and the target page.
+   */
+  force?: boolean
 }
 
 export interface BeforeSave {

diff --git a/superglue/spec/features/navigation.spec.jsx b/superglue/spec/features/navigation.spec.jsx
@@ -650,7 +650,10 @@ describe('navigation', () => {
       )
       const store = instance.store
 
-      const mockResponse = rsp.graftSuccessWithNewZip()
+      const mockResponse = rsp.graftSuccessWithNewZip({
+        componentIdentifier: 'home',
+      })
+
       mockResponse.headers['x-response-url'] = '/about'
       fetchMock.mock(
         'http://example.com/about?props_at=address&format=json',

diff --git a/superglue/spec/fixtures.js b/superglue/spec/fixtures.js
@@ -14,7 +14,7 @@ export const visitSuccess = () => {
   }
 }
 
-export const graftSuccessWithNewZip = () => {
+export const graftSuccessWithNewZip = (body) => {
   return {
     body: JSON.stringify({
       data: { zip: 91210 },
@@ -23,6 +23,7 @@ export const graftSuccessWithNewZip = () => {
       csrfToken: 'token',
       assets: ['application-new123.js', 'application-new123.js'],
       fragments: [],
+      ...body,
     }),
     headers: {
       'content-type': 'application/json',

diff --git a/superglue/spec/lib/action_creators.spec.js b/superglue/spec/lib/action_creators.spec.js
@@ -14,6 +14,7 @@ import * as helpers from '../../lib/utils/helpers'
 import * as rsp from '../../spec/fixtures'
 import { configureStore } from '@reduxjs/toolkit'
 import { rootReducer } from '../../lib'
+import { MismatchedComponentError } from '../../lib/action_creators'
 
 const buildStore = (preloadedState) => {
   let resultsReducer = (state = [], action) => {
@@ -1133,8 +1134,7 @@ describe('action creators', () => {
         store.dispatch(remote('/foo', { pageKey: '/foo' }))
       }))
 
-    it('warns if a received page has a completely component id that the target page it will replace', () => {
-      vi.spyOn(console, 'warn')
+    it('throws if a received page has a completely component id that the target page it will replace', () => {
       const store = buildStore({
         superglue: {
           currentPageKey: '/bar',
@@ -1164,18 +1164,46 @@ describe('action creators', () => {
         },
       })
 
-      return store.dispatch(remote('/bar')).then(() => {
-        expect(console.warn).toHaveBeenCalledWith(
-          `You're about replace an existing page located at pages["/bar"]
-that has the componentIdentifier "bar-id" with the contents of a
-received page that has a componentIdentifier of "foo-id".
+      expect(() => store.dispatch(remote('/bar'))).rejects.toThrow(
+        new MismatchedComponentError('bar-id', 'foo-id', '/bar')
+      )
+    })
 
-This can happen if you're using data-sg-remote or remote but your response
-redirected to a completely different page. Since remote requests do not
-navigate or change the current page component, your current page component may
-receive a shape that is unexpected and cause issues with rendering.
+    it('forces a remote even if a received page has a completely component id than the target page', () => {
+      const store = buildStore({
+        superglue: {
+          currentPageKey: '/bar',
+          csrfToken: 'token',
+        },
+        pages: {
+          '/bar': {
+            data: {},
+            componentIdentifier: 'bar-id',
+          },
+        },
+      })
 
-Consider using data-sg-visit, the visit function, or redirect_back.`
+      const successfulBody = {
+        data: { greeting: 'hello' },
+        componentIdentifier: 'foo-id',
+        csrfToken: 'token',
+        assets: [],
+        defers: [],
+        fragments: [],
+      }
+
+      fetchMock.mock('/bar?format=json', {
+        body: successfulBody,
+        headers: {
+          'content-type': 'application/json',
+        },
+      })
+
+      return store.dispatch(remote('/bar', { force: true })).then(() => {
+        expect(store.getState().pages['/bar'].data).toEqual(
+          expect.objectContaining({
+            greeting: 'hello',
+          })
         )
       })
     })