Merge url query with exportPathMap

[lucleray]

This PR fixes #4615

From the issue :

One thing we might consider is merging and showing a warning for keys not defined in exportPathMap

The behaviour after this PR is the following :

// next.config.js
module.exports = {
  exportPathMap: () => ({
    '/': { page: '/', query: { a: 'blue' } }
  })
}

url called	ctx.query	warning ?
/	{ a: 'blue' }
/?a=red	{ a: 'blue' }
/?b=green	{ a: 'blue', b: 'green' }	... parameter 'b' missing in exportPathMap

Is that the expected behaviour ? If not, I'll update the PR to shape the expected behavior.

[timneutkens]

Let's do {...urlQuery, ...query} instead of Object.assign

[timneutkens]

Let's add a test for this new behavior 👍 can be added in test/integration/export

[timneutkens]

My review was actually a request changes

[timneutkens]

I actually gave the wrong directory, static already has tests for next export. It also has a test for the query I believe 🤔 So you can add the extra tests there. Sorry for the confusion 🙏

[lucleray]

@timneutkens I added the tests for the 3 scenario in the PR's description.

I ran into the following problems while writing the tests :

1. launchApp / killApp

If I used launchApp from the next-test-utils, it was harder to catch console.warn call and the app wouldn't shut down properly.

killApp does kill the PID of the spawned process, here :
https://github.com/zeit/next.js/blob/7fcfb8bde91af096af6905ec8c426934f667f0b6/test/lib/next-test-utils.js#L102-L104

But it does not kill the PID of the "subprocess" spawned by the spawned process 💣
Basically, launchApp spawn a new process : next, which spawns another new process : next-dev
They get PID and PID+1 (for example 44013 and 44014).
killApp only kills PID, but not PID+1. They would need to be both killed for killApp to work properly.

2. startApp / stopApp

There is a bug in stopApp, I think.

More precisely here :

https://github.com/zeit/next.js/blob/7fcfb8bde91af096af6905ec8c426934f667f0b6/test/lib/next-test-utils.js#L116-L121

app is never touched.

What I did

So I ended up simply using start and stop from server/next. Not sure it's the best thing to do for integration test. Let me know 🙂

[lucleray]

@timneutkens
Ok, I can move the tests to static

Oh, but static runs tests with a static server. These tests need to be run with a dev server.
EDIT : my bad, it has both, I'll move it there 👍

[timneutkens]

You actually don't have to test the console.log calls, just check loading the page with a querystring and query defined and see if it works. We don't test console.log side effects as they will most likely change in the future and can't break anything.

[lucleray]

I fixed the tests, I removed the test for console.log.

[timneutkens]

Looks great 👍