From ab6c1935127a2968fd3666c50ce17b7df0168ccc Mon Sep 17 00:00:00 2001
From: Carlos Cuesta <carloscuesta@me.com>
Date: Thu, 7 Jan 2021 14:19:13 +0100
Subject: [PATCH] :bug: Do not update querystring on search

After trying this out I think its not a great pattern. If we want to implement this we need to make it onBlur or onSubmit, but definitely not onChange. This causes a weird bug where the page is scrolled to the top since were using router.push and also affects browser navigateBack history. Since the push method modifies the history.pushState.
---
 src/__tests__/pages.spec.js                   |  1 -
 .../GitmojiList/__tests__/gitmojiList.spec.js | 62 +++++--------------
 src/components/GitmojiList/index.js           |  8 +--
 3 files changed, 20 insertions(+), 51 deletions(-)

diff --git a/src/__tests__/pages.spec.js b/src/__tests__/pages.spec.js
index 8f647e2..82fb635 100644
--- a/src/__tests__/pages.spec.js
+++ b/src/__tests__/pages.spec.js
@@ -12,7 +12,6 @@ jest.mock('next/router', () => ({
   events: { on: jest.fn(), off: jest.fn() },
   useRouter: jest.fn().mockImplementation(() => ({
     query: {},
-    push: jest.fn(),
   })),
 }))
 
diff --git a/src/components/GitmojiList/__tests__/gitmojiList.spec.js b/src/components/GitmojiList/__tests__/gitmojiList.spec.js
index 3051812..8831cad 100644
--- a/src/components/GitmojiList/__tests__/gitmojiList.spec.js
+++ b/src/components/GitmojiList/__tests__/gitmojiList.spec.js
@@ -38,7 +38,7 @@ describe('GitmojiList', () => {
       Router.useRouter.mockReturnValue(stubs.routerMock())
     })
 
-    it('should find the fire gitmoji by code and update router.query', () => {
+    it('should find the fire gitmoji by code', () => {
       const wrapper = renderer.create(<GitmojiList {...stubs.props} />)
       const instance = wrapper.root
       const query = 'Fire'
@@ -49,15 +49,10 @@ describe('GitmojiList', () => {
           .props.onChange({ target: { value: query } })
       })
 
-      expect(Router.useRouter().push).toHaveBeenLastCalledWith(
-        { query: { search: query } },
-        undefined,
-        { shallow: true }
-      )
       expect(instance.findAllByType('article').length).toEqual(1)
     })
 
-    it('should find the fire gitmoji by description and update router.query', () => {
+    it('should find the fire gitmoji by description', () => {
       const wrapper = renderer.create(<GitmojiList {...stubs.props} />)
       const instance = wrapper.root
       const query = 'remove'
@@ -68,43 +63,10 @@ describe('GitmojiList', () => {
           .props.onChange({ target: { value: query } })
       })
 
-      expect(Router.useRouter().push).toHaveBeenLastCalledWith(
-        { query: { search: query } },
-        undefined,
-        { shallow: true }
-      )
       expect(instance.findAllByType('article').length).toEqual(1)
     })
   })
 
-  describe('when user deletes the search query', () => {
-    beforeAll(() => {
-      Router.useRouter.mockReturnValue(stubs.routerMock())
-    })
-
-    it('should clear the router.query', () => {
-      const wrapper = renderer.create(<GitmojiList {...stubs.props} />)
-      const instance = wrapper.root
-      const query = 'Fire'
-
-      renderer.act(() => {
-        instance
-          .findByType('input')
-          .props.onChange({ target: { value: query } })
-      })
-
-      renderer.act(() => {
-        instance.findByType('input').props.onChange({ target: { value: '' } })
-      })
-
-      expect(Router.useRouter().push).toHaveBeenLastCalledWith(
-        { query: {} },
-        undefined,
-        { shallow: true }
-      )
-    })
-  })
-
   describe('when search is provided by query string', () => {
     beforeAll(() => {
       Router.useRouter.mockReturnValue(stubs.routerMock({ search: 'fire' }))
@@ -118,12 +80,22 @@ describe('GitmojiList', () => {
         wrapper.update(<GitmojiList {...stubs.props} />)
       })
 
-      expect(Router.useRouter().push).toHaveBeenCalledWith(
-        { query: { search: query } },
-        undefined,
-        { shallow: true }
-      )
       expect(wrapper.root.findByType('input').props.value).toEqual(query)
     })
+
+    describe('when the user deletes the search input', () => {
+      it('should clear the query string', () => {
+        const wrapper = renderer.create(<GitmojiList {...stubs.props} />)
+        const instance = wrapper.root
+
+        renderer.act(() => {
+          instance.findByType('input').props.onChange({ target: { value: '' } })
+        })
+
+        expect(Router.useRouter().push).toHaveBeenCalledWith('/', undefined, {
+          shallow: true,
+        })
+      })
+    })
   })
 })
diff --git a/src/components/GitmojiList/index.js b/src/components/GitmojiList/index.js
index e55d62a..d02d02f 100644
--- a/src/components/GitmojiList/index.js
+++ b/src/components/GitmojiList/index.js
@@ -38,11 +38,9 @@ const GitmojiList = (props: Props): Element<'div'> => {
   }, [router.query.search])
 
   React.useEffect(() => {
-    router.push(
-      { query: searchInput ? { search: searchInput } : {} },
-      undefined,
-      { shallow: true }
-    )
+    if (router.query.search && !searchInput) {
+      router.push('/', undefined, { shallow: true })
+    }
   }, [searchInput])
 
   React.useEffect(() => {