From 449f38daa7f7f67fbe66606c246efc0449ffce17 Mon Sep 17 00:00:00 2001
From: Tim Neutkens <tim@timneutkens.nl>
Date: Sat, 12 May 2018 20:10:17 +0200
Subject: [PATCH] Make sure props.url is immutable (#4352)

* Make sure props.url is immutable

* Add test for immutable url

* Match object instead of string
---
 lib/app.js                                    |  8 ++--
 .../basic/pages/nav/url-prop-change.js        | 37 +++++++++++++++++++
 .../basic/test/client-navigation.js           | 14 +++++++
 test/integration/basic/test/index.test.js     |  1 +
 4 files changed, 57 insertions(+), 3 deletions(-)
 create mode 100644 test/integration/basic/pages/nav/url-prop-change.js

diff --git a/lib/app.js b/lib/app.js
index ebee26d13970b..7cecba362e36a 100644
--- a/lib/app.js
+++ b/lib/app.js
@@ -93,18 +93,20 @@ const warnUrl = execOnce(() => {
 })
 
 export function createUrl (router) {
+  // This is to make sure we don't references the router object at call time
+  const {pathname, asPath, query} = router
   return {
     get query () {
       warnUrl()
-      return router.query
+      return query
     },
     get pathname () {
       warnUrl()
-      return router.pathname
+      return pathname
     },
     get asPath () {
       warnUrl()
-      return router.asPath
+      return asPath
     },
     back: () => {
       warnUrl()
diff --git a/test/integration/basic/pages/nav/url-prop-change.js b/test/integration/basic/pages/nav/url-prop-change.js
new file mode 100644
index 0000000000000..79715a087db61
--- /dev/null
+++ b/test/integration/basic/pages/nav/url-prop-change.js
@@ -0,0 +1,37 @@
+import React from 'react'
+import Link from 'next/link'
+
+export default class UrlPropChange extends React.Component {
+  constructor (props) {
+    super(props)
+    this.state = {
+      previousUrl: {},
+      url: props.url
+    }
+  }
+
+  componentWillReceiveProps (nextProps) {
+    this.setState(() => {
+      return {
+        previousUrl: this.props.url,
+        url: nextProps.url
+      }
+    })
+  }
+
+  render () {
+    const {previousUrl, url} = this.state
+    return <div>
+      Current:
+      <div id='url-result'>
+        {JSON.stringify(url)}
+      </div>
+      <br /><br />
+      Previous:
+      <div id='previous-url-result'>
+        {JSON.stringify(previousUrl)}
+      </div>
+      <Link href='/nav/url-prop-change?added=yes'><a id='add-query'>Add querystring</a></Link>
+    </div>
+  }
+}
diff --git a/test/integration/basic/test/client-navigation.js b/test/integration/basic/test/client-navigation.js
index 192cc523f845d..55b0d3b5218a4 100644
--- a/test/integration/basic/test/client-navigation.js
+++ b/test/integration/basic/test/client-navigation.js
@@ -33,6 +33,20 @@ export default (context, render) => {
       })
     })
 
+    describe('With url property', () => {
+      it('Should keep immutable pathname, asPath and query', async () => {
+        const browser = await webdriver(context.appPort, '/nav/url-prop-change')
+        await browser.elementByCss('#add-query').click()
+        const urlResult = await browser.elementByCss('#url-result').text()
+        const previousUrlResult = await browser.elementByCss('#previous-url-result').text()
+
+        expect(JSON.parse(urlResult)).toMatchObject({'query': {'added': 'yes'}, 'pathname': '/nav/url-prop-change', 'asPath': '/nav/url-prop-change?added=yes'})
+        expect(JSON.parse(previousUrlResult)).toMatchObject({'query': {}, 'pathname': '/nav/url-prop-change', 'asPath': '/nav/url-prop-change'})
+
+        browser.close()
+      })
+    })
+
     describe('with <a/> tag inside the <Link />', () => {
       it('should navigate the page', async () => {
         const browser = await webdriver(context.appPort, '/nav/about')
diff --git a/test/integration/basic/test/index.test.js b/test/integration/basic/test/index.test.js
index 751618175da90..d79a2684a4935 100644
--- a/test/integration/basic/test/index.test.js
+++ b/test/integration/basic/test/index.test.js
@@ -52,6 +52,7 @@ describe('Basic Features', () => {
       renderViaHTTP(context.appPort, '/nav/redirect'),
       renderViaHTTP(context.appPort, '/nav/as-path'),
       renderViaHTTP(context.appPort, '/nav/as-path-using-router'),
+      renderViaHTTP(context.appPort, '/nav/url-prop-change'),
 
       renderViaHTTP(context.appPort, '/nested-cdm/index'),