`shallowEquals` throws when using custom server and not passing an object for query to `app.render`

[migueloller]

• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Calling app.render(req, res, pathname, query) should not cause any issues.

Current Behavior

If app.render(req, res, pathname, query) is called and query is not an object, there will be a nasty error that will happen when calling Router.push (or any other router method that internally calls change).

Steps to Reproduce (for bugs)

1. Set up a custom Next.js server
2. Call app.render while passing an undefined value for query.
3. Call Router.push with the same pathname and a query.

Context

I was trying to move from a dynamic server to using static exports and had to deal with this bug for about two hours. I've gone deep into Next.js's source code so I can help pinpoint the exact path the bug follows. Basically, it's something like app.render -> renderToHTML -> doRender -> new Router.

To solve this issue it's as simple as modifying the shallow comparison function to be a bit more safe. It should check that both inputs are objects and if not return false. For now, I'm just doing the following:

server.get('*', (req, res, n) => {
  const { pathname } = parse(req.url, true)
  const route = routes[pathname]
  if (route) return app.render(req, res, route.page, route.query || {})
  n()
})

This example in the wiki should be updated.

Your Environment

Tech	Version
next	3.2.2
node	8.4.0
OS	macOS 10.12.6
browser	Chrome 60.0.3112.113

[timneutkens]

It should actually throw an error when one of either in shallow equals is not an object. Feel free to send a PR.

I've updated the wiki!

[timneutkens]

Lets actually solve this by checking if query is not an object, and then throwing an error, to enforce always providing all arguments.

[migueloller]

@timneutkens, that would work!

Do you think the more correct approach would be to throw an error on the shallow equals as well? What’s the contract for the function (i.e., type)? Does it take any two arguments or is it two objects. If it’s two objects then throw an error if any isn’t an object. If it’s anything then return false if their “typeof”s are different.

Then also throwing when query isn’t an object is a good idea.

[timneutkens]

Yeah like I said in my first comment we should add that, but also add a check for the querystring to be an object.

[chrislam]

The calling code in the urlIsNew method of the Router seems to pass through whatever is parsed from the url, should it be ensuring that an object is passed through instead?