From 3a5ab37cc28d945cf2fbe92417dfe0d0e3d0ef78 Mon Sep 17 00:00:00 2001 From: Alex Kholodniak Date: Sat, 22 Jun 2024 19:04:26 -0500 Subject: [PATCH 1/3] Add data-sg-replace functionality --- superglue/lib/action_creators/requests.ts | 4 +- superglue/lib/utils/ujs.ts | 13 ++++- superglue/spec/lib/utils/ujs.spec.js | 66 +++++++++++++++++++---- 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/superglue/lib/action_creators/requests.ts b/superglue/lib/action_creators/requests.ts index 4990232e..d3d60fd1 100644 --- a/superglue/lib/action_creators/requests.ts +++ b/superglue/lib/action_creators/requests.ts @@ -152,6 +152,7 @@ export function visit( placeholderKey, beforeSave = (prevPage, receivedPage) => receivedPage, revisit = false, + suggestedAction = 'push', } = {} ) { path = withoutBusters(path) @@ -208,7 +209,8 @@ export function visit( } const isGet = fetchArgs[1].method === 'GET' - meta.suggestedAction = 'push' + meta.suggestedAction = suggestedAction + if (!rsp.redirected && !isGet) { meta.suggestedAction = 'replace' } diff --git a/superglue/lib/utils/ujs.ts b/superglue/lib/utils/ujs.ts index 93cfd2c1..7dadf69d 100644 --- a/superglue/lib/utils/ujs.ts +++ b/superglue/lib/utils/ujs.ts @@ -48,8 +48,11 @@ export class HandlerBuilder { const hasRemote = !!node.getAttribute( this.attributePrefix + '-remote' ) + const hasReplace = !!node.getAttribute( + this.attributePrefix + '-replace' + ) - return hasVisit || hasRemote + return hasVisit || hasRemote || hasReplace } handleSubmit(event) { @@ -92,6 +95,7 @@ export class HandlerBuilder { visitOrRemote(linkOrForm, url, opts: any = {}) { let target + let suggestedAction = 'push' if (linkOrForm.getAttribute(this.attributePrefix + '-visit')) { target = this.visit @@ -107,10 +111,15 @@ export class HandlerBuilder { target = this.remote } + if (linkOrForm.getAttribute(this.attributePrefix + '-replace')) { + target = this.visit + suggestedAction = 'replace' + } + if (!target) { return } else { - return target(url, opts) + return target(url, { ...opts, suggestedAction }) } } diff --git a/superglue/spec/lib/utils/ujs.spec.js b/superglue/spec/lib/utils/ujs.spec.js index 7e76bb45..319e8fa9 100644 --- a/superglue/spec/lib/utils/ujs.spec.js +++ b/superglue/spec/lib/utils/ujs.spec.js @@ -68,6 +68,25 @@ describe('ujs', () => { } } + function createFakeReplaceEvent() { + return { + preventDefault: () => {}, + target: { + nodeName: 'A', + parentNode: 'DIV', + href: '/foo', + getAttribute: (attr) => { + if(attr === 'href') { + return '/foo' + } + if(attr === 'data-replace') { + return true + } + } + } + } + } + describe('onClick', () => { it('calls visit on a valid link', () => { const ujsAttributePrefix = 'data' @@ -89,7 +108,7 @@ describe('ujs', () => { const {onClick} = builder.handlers() onClick(createFakeEvent()) - expect(visit).toHaveBeenCalledWith('/foo', {method: 'GET'}) + expect(visit).toHaveBeenCalledWith('/foo', {method: 'GET', suggestedAction: 'push'}) }) it('calls visit with a placeholder when props_at is present on a valid link', () => { @@ -120,7 +139,11 @@ describe('ujs', () => { const {onClick} = builder.handlers() onClick(createFakeVisitGraftEvent()) - expect(visit).toHaveBeenCalledWith('/foo?props_at=data.hello', {method: 'GET', placeholderKey: '/current'}) + expect(visit).toHaveBeenCalledWith('/foo?props_at=data.hello', { + method: 'GET', + placeholderKey: '/current', + suggestedAction: 'push', + }) }) it('calls remote if a link is enabled with remote', () => { @@ -143,10 +166,33 @@ describe('ujs', () => { const {onClick} = builder.handlers() onClick(createFakeRemoteEvent()) - expect(remote).toHaveBeenCalledWith('/foo', {method: 'GET'}) + expect(remote).toHaveBeenCalledWith('/foo', {method: 'GET', suggestedAction: 'push'}) + }) + + it('calls visit with replace action if link has data-replace attribute', () => { + const ujsAttributePrefix = 'data' + const visit = jest.fn() + const navigatorRef = { + current: { + navigateTo: () => {} + } + } + const store = {} + + const builder = new HandlerBuilder({ + ujsAttributePrefix, + store, + visit, + navigatorRef + }) + + const {onClick} = builder.handlers() + onClick(createFakeReplaceEvent()) + + expect(visit).toHaveBeenCalledWith('/foo', {method: 'GET', suggestedAction: 'replace'}) }) - it('does not call visit on an link does not have the visit attribute data-visit', () => { + it('does not call visit on a link that does not have the visit attribute data-visit', () => { const store = {} const ujsAttributePrefix = 'data' const visit = jest.fn() @@ -176,7 +222,7 @@ describe('ujs', () => { expect(visit).not.toHaveBeenCalledWith('/foo', {}) }) - it('does not call visit on an non-standard link', () => { + it('does not call visit on a non-standard link', () => { const store = {} const ujsAttributePrefix = 'data' const visit = jest.fn() @@ -227,7 +273,7 @@ describe('ujs', () => { fakeEvent = createFakeEvent() onClick(fakeEvent) - expect(visit).toHaveBeenCalledWith('/foo', {method: 'GET'}) + expect(visit).toHaveBeenCalledWith('/foo', {method: 'GET', suggestedAction: 'push'}) }) }) @@ -274,7 +320,7 @@ describe('ujs', () => { } } - it('succssfully posts a form with a visit attribute', () => { + it('successfully posts a form with a visit attribute', () => { const store = {} const ujsAttributePrefix = 'data' const visit = jest.fn() @@ -303,11 +349,12 @@ describe('ujs', () => { headers: { "content-type": null, }, + suggestedAction: 'push', body: {some: 'Body'} }) }) - it('succssfully posts a form with a remote attribut', () => { + it('successfully posts a form with a remote attribute', () => { const store = {} const ujsAttributePrefix = 'data' const remote = jest.fn() @@ -336,11 +383,12 @@ describe('ujs', () => { headers: { "content-type": null, }, + suggestedAction: 'push', body: {some: 'Body'} }) }) - it('does not posts a form without a visit attribute', () => { + it('does not post a form without a visit attribute', () => { const store = {} const ujsAttributePrefix = 'data' const visit = jest.fn() From c379297da66b0b783bee6367c4df5eff76731c8d Mon Sep 17 00:00:00 2001 From: Alex Kholodniak Date: Mon, 24 Jun 2024 18:27:25 -0500 Subject: [PATCH 2/3] comments fixes --- superglue/lib/action_creators/requests.ts | 11 ++++++--- superglue/lib/utils/ujs.ts | 21 +++++++++-------- superglue/spec/lib/action_creators.spec.js | 2 +- superglue/spec/lib/utils/ujs.spec.js | 26 +--------------------- 4 files changed, 20 insertions(+), 40 deletions(-) diff --git a/superglue/lib/action_creators/requests.ts b/superglue/lib/action_creators/requests.ts index d3d60fd1..a40b087f 100644 --- a/superglue/lib/action_creators/requests.ts +++ b/superglue/lib/action_creators/requests.ts @@ -152,7 +152,7 @@ export function visit( placeholderKey, beforeSave = (prevPage, receivedPage) => receivedPage, revisit = false, - suggestedAction = 'push', + action = 'push', } = {} ) { path = withoutBusters(path) @@ -175,7 +175,7 @@ export function visit( if (!placeholderKey && hasPropsAt(path)) { console.warn( - `visit was called with props_at param in the path ${path}, this will be ignore unless you provide a placeholder.` + `visit was called with props_at param in the path ${path}, this will be ignored unless you provide a placeholder.` ) path = removePropsAt(path) } @@ -207,9 +207,14 @@ export function visit( rsp, fetchArgs, } + const isGet = fetchArgs[1].method === 'GET' - meta.suggestedAction = suggestedAction + if (action !== undefined) { + meta.suggestedAction = action + } else { + meta.suggestedAction = 'push' + } if (!rsp.redirected && !isGet) { meta.suggestedAction = 'replace' diff --git a/superglue/lib/utils/ujs.ts b/superglue/lib/utils/ujs.ts index 7dadf69d..77ea38e0 100644 --- a/superglue/lib/utils/ujs.ts +++ b/superglue/lib/utils/ujs.ts @@ -48,11 +48,8 @@ export class HandlerBuilder { const hasRemote = !!node.getAttribute( this.attributePrefix + '-remote' ) - const hasReplace = !!node.getAttribute( - this.attributePrefix + '-replace' - ) - return hasVisit || hasRemote || hasReplace + return hasVisit || hasRemote } handleSubmit(event) { @@ -95,7 +92,6 @@ export class HandlerBuilder { visitOrRemote(linkOrForm, url, opts: any = {}) { let target - let suggestedAction = 'push' if (linkOrForm.getAttribute(this.attributePrefix + '-visit')) { target = this.visit @@ -105,21 +101,24 @@ export class HandlerBuilder { if (placeholderKey) { opts.placeholderKey = urlToPageKey(placeholderKey) } + + if ( + linkOrForm.getAttribute(this.attributePrefix + '-replace') + ) { + opts.suggestedAction = 'replace' + } else { + opts.suggestedAction = 'push' + } } if (linkOrForm.getAttribute(this.attributePrefix + '-remote')) { target = this.remote } - if (linkOrForm.getAttribute(this.attributePrefix + '-replace')) { - target = this.visit - suggestedAction = 'replace' - } - if (!target) { return } else { - return target(url, { ...opts, suggestedAction }) + return target(url, opts) } } diff --git a/superglue/spec/lib/action_creators.spec.js b/superglue/spec/lib/action_creators.spec.js index 1d16cf37..9a71b0bd 100644 --- a/superglue/spec/lib/action_creators.spec.js +++ b/superglue/spec/lib/action_creators.spec.js @@ -1388,7 +1388,7 @@ Consider using data-sg-visit, the visit function, or redirect_back.` const expectedFetchUrl = '/first?props_at=foo' store.dispatch(visit(expectedFetchUrl)).then((meta) => { expect(console.warn).toHaveBeenCalledWith( - 'visit was called with props_at param in the path /first?props_at=foo, this will be ignore unless you provide a placeholder.' + 'visit was called with props_at param in the path /first?props_at=foo, this will be ignored unless you provide a placeholder.' ) done() }) diff --git a/superglue/spec/lib/utils/ujs.spec.js b/superglue/spec/lib/utils/ujs.spec.js index 319e8fa9..a80f0830 100644 --- a/superglue/spec/lib/utils/ujs.spec.js +++ b/superglue/spec/lib/utils/ujs.spec.js @@ -166,30 +166,7 @@ describe('ujs', () => { const {onClick} = builder.handlers() onClick(createFakeRemoteEvent()) - expect(remote).toHaveBeenCalledWith('/foo', {method: 'GET', suggestedAction: 'push'}) - }) - - it('calls visit with replace action if link has data-replace attribute', () => { - const ujsAttributePrefix = 'data' - const visit = jest.fn() - const navigatorRef = { - current: { - navigateTo: () => {} - } - } - const store = {} - - const builder = new HandlerBuilder({ - ujsAttributePrefix, - store, - visit, - navigatorRef - }) - - const {onClick} = builder.handlers() - onClick(createFakeReplaceEvent()) - - expect(visit).toHaveBeenCalledWith('/foo', {method: 'GET', suggestedAction: 'replace'}) + expect(remote).toHaveBeenCalledWith('/foo', {method: 'GET'}) }) it('does not call visit on a link that does not have the visit attribute data-visit', () => { @@ -383,7 +360,6 @@ describe('ujs', () => { headers: { "content-type": null, }, - suggestedAction: 'push', body: {some: 'Body'} }) }) From 2bb8ce08d14e640e90334947c350b28684d1cb9e Mon Sep 17 00:00:00 2001 From: Alex Kholodniak Date: Tue, 25 Jun 2024 18:54:05 -0500 Subject: [PATCH 3/3] comments fixes & tests --- superglue/lib/action_creators/requests.ts | 12 ++-- superglue/spec/lib/utils/ujs.spec.js | 83 ++++++++++++++++++++++- 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/superglue/lib/action_creators/requests.ts b/superglue/lib/action_creators/requests.ts index a40b087f..d6d93fc9 100644 --- a/superglue/lib/action_creators/requests.ts +++ b/superglue/lib/action_creators/requests.ts @@ -152,7 +152,7 @@ export function visit( placeholderKey, beforeSave = (prevPage, receivedPage) => receivedPage, revisit = false, - action = 'push', + action, } = {} ) { path = withoutBusters(path) @@ -210,11 +210,7 @@ export function visit( const isGet = fetchArgs[1].method === 'GET' - if (action !== undefined) { - meta.suggestedAction = action - } else { - meta.suggestedAction = 'push' - } + meta.suggestedAction = 'push' if (!rsp.redirected && !isGet) { meta.suggestedAction = 'replace' @@ -228,6 +224,10 @@ export function visit( } } + if (action) { + meta.suggestedAction = action + } + pageKey = urlToPageKey(rsp.url) if (!isGet && !rsp.redirected) { diff --git a/superglue/spec/lib/utils/ujs.spec.js b/superglue/spec/lib/utils/ujs.spec.js index a80f0830..e07a142d 100644 --- a/superglue/spec/lib/utils/ujs.spec.js +++ b/superglue/spec/lib/utils/ujs.spec.js @@ -79,6 +79,9 @@ describe('ujs', () => { if(attr === 'href') { return '/foo' } + if(attr === 'data-visit') { + return true + } if(attr === 'data-replace') { return true } @@ -169,6 +172,32 @@ describe('ujs', () => { expect(remote).toHaveBeenCalledWith('/foo', {method: 'GET'}) }) + it('calls visit with replace action if link has data-replace attribute', () => { + const ujsAttributePrefix = 'data' + const visit = jest.fn() + const navigatorRef = { + current: { + navigateTo: () => {}, + }, + } + const store = {} + + const builder = new HandlerBuilder({ + ujsAttributePrefix, + store, + visit, + navigatorRef, + }) + + const { onClick } = builder.handlers() + onClick(createFakeReplaceEvent()) + + expect(visit).toHaveBeenCalledWith('/foo', { + method: 'GET', + suggestedAction: 'replace', + }) + }) + it('does not call visit on a link that does not have the visit attribute data-visit', () => { const store = {} const ujsAttributePrefix = 'data' @@ -400,7 +429,59 @@ describe('ujs', () => { expect(visit).not.toHaveBeenCalledWith('/foo', { method: 'POST', - body: {some: 'Body'} + body: { some: 'Body' }, + }) + }) + + it('calls visit with replace action if form has data-replace attribute', () => { + const store = {} + const ujsAttributePrefix = 'data' + const visit = jest.fn() + const navigatorRef = { + current: { + navigateTo: () => {}, + }, + } + + const builder = new HandlerBuilder({ + ujsAttributePrefix, + store, + visit, + navigatorRef, + }) + global.FormData = () => {} + jest + .spyOn(global, 'FormData') + .mockImplementation(() => ({ some: 'Body' })) + + const { onSubmit } = builder.handlers() + const fakeFormEvent = createFakeFormEvent() + fakeFormEvent.target.getAttribute = (attr) => { + if (attr === 'action') { + return '/foo' + } + if (attr === 'method') { + return 'POST' + } + if (attr === 'data-visit') { + return true + } + if (attr === 'data-replace') { + return true + } + } + onSubmit(fakeFormEvent) + + expect(global.FormData).toHaveBeenCalledWith( + fakeFormEvent.target + ) + expect(visit).toHaveBeenCalledWith('/foo', { + method: 'POST', + headers: { + 'content-type': null, + }, + suggestedAction: 'replace', + body: { some: 'Body' }, }) }) })