Skip to content

Commit

Permalink
Add warning for remote reqs (#29)
Browse files Browse the repository at this point in the history
This adds a warning to remote when it tries to replace a page that has a different
componentIdentifier that a received page. This can happen when you redirect to a
completely new page using the current page's url via a form submit. And since remote
doesn't navigate away from the current page, the current page component may receive
props that are shaped differently.
  • Loading branch information
jho406 authored Nov 25, 2023
1 parent 1c56376 commit e087e60
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 0 deletions.
23 changes: 23 additions & 0 deletions superglue/lib/action_creators/requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,29 @@ export function remote(
fetchArgs,
}

const willReplaceCurrent = pageKey == currentPageKey
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.`
)
}

const page = beforeSave(pages[pageKey], json)
return dispatch(saveAndProcessPage(pageKey, page)).then(
() => {
Expand Down
87 changes: 87 additions & 0 deletions superglue/spec/lib/action_creators.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,93 @@ 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', () => {
jest.spyOn(console, 'warn')
const store = mockStore({
superglue: {
currentPageKey: '/bar',
csrfToken: 'token',
},
pages: {
"/bar": {
data: {},
componentIdentifier: "bar-id"
}
}
})

const successfulBody = {
data: {},
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'))
.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".
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.`
)
})
})

it('does not warn if a received page is not replacing a target page with a different componentIdentifier', () => {
jest.spyOn(console, 'warn')
const store = mockStore({
superglue: {
currentPageKey: '/bar',
csrfToken: 'token',
},
pages: {
"/bar": {
data: {},
componentIdentifier: "ForBar"
}
}
})

const successfulBody = {
data: {},
componentIdentifier: "ForBar",
csrfToken: 'token',
assets: [],
defers: [],
fragments: [],
}

fetchMock.mock('/bar?format=json', {
body: successfulBody,
headers: {
'content-type': 'application/json',
},
})

return store
.dispatch(remote('/bar'))
.then(() => {
expect(console.warn).not.toHaveBeenCalled()
})
})
})

describe('visit', () => {
Expand Down

0 comments on commit e087e60

Please sign in to comment.