router client file is transpiled incorrectly

[arackaf]

Verify canary release

• I verified that the issue exists in Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 21.4.0: Mon Feb 21 20:34:37 PST 2022; root:xnu-8020.101.4~2/RELEASE_X86_64
Binaries:
  Node: 16.11.1
  npm: 8.0.0
  Yarn: 1.22.17
  pnpm: N/A
Relevant packages:
  next: 12.1.5
  react: 17.0.2
  react-dom: 17.0.2

What browser are you using? (if relevant)

n/a

How are you deploying your application? (if relevant)

n/a

Describe the Bug

The dist/client/router.js file had this code added in version 12.1.5

if (typeof exports.default === 'function' || (typeof exports.default === 'object' && exports.default !== null)) {
  Object.assign(exports.default, exports);
  module.exports = exports.default;
}

This essentially erases all named exports from the CJS exports object, and instead hangs them all on the default export. This breaks Jest code that's mocking the useRouter hook.

import * as routerModule from 'next/router';

// ...

jest.spyOn(routerModule, 'useRouter').mockReturnValue(({
  pathname: '/foo/bar',
  push: pushMockFn,
} as unknown) as useRouter.NextRouter);

now breaks, since routerModule.useRouter is null, requiring instead

import routerModule from 'next/router';

This fixes the test, but is wrong. useRouter is not a property on the default export (or shouldn't be) but is rather a named export in that module.

Expected Behavior

CJS exports should match ESM.

To Reproduce

See above

[Brooooooklyn]

This essentially erases all named exports from the CJS exports object, and instead hangs them all on the default export.

Actually it will keep everything on exports.

Since we are discussing CJS import exports here, and your code here is ESM style, could you please provide your jest config as well, it may issue in jest transformers.

[arackaf]

Actually it will keep everything on exports.

It doesn't. Look closely at the new code

if (typeof exports.default === 'function' || (typeof exports.default === 'object' && exports.default !== null)) {
  Object.assign(exports.default, exports);
  module.exports = exports.default;
}

The first line in the if block smears all named exports onto the default export; the second line makes the default export be the only thing existing on exports

That's why

import * as routerModule from 'next/router';

routerModule.useRouter // is now null

but

import routerModule from 'next/router';

routerModule.useRouter // works but shouldn't

This does not match the esm structure of this module, causing code to break.

As for configuration, we have this in our Jest config

transform: {
  '^.+\\.(js|jsx|mjs|cjs|ts|tsx)$': require.resolve(
    '@mrkt/jest/transforms/babel',
  ),
  '^.+\\.css$': require.resolve('@mrkt/jest/transforms/css'),
  '^(?!.*\\.(js|jsx|mjs|cjs|ts|tsx|css|json)$)': require.resolve(
    '@mrkt/jest/transforms/file',
  ),
},

and we have babel-jest version 24.9.0 installed.

[Brooooooklyn]

The reason why

import * as routerModule from 'next/router';

routerModule.useRouter // is now null

is because of the transformer, not the code itself. If you are using native require, it works fine:

const routerModule = require('next/router');
console.log(routerModule.createRouter) // [Function: createRouter]

const { createRouter } = require('next/router');
console.log(createRouter) // [Function: createRouter]

[arackaf]

I understand it's the transformer. That's what this entire issue is about. The transformer is breaking Jest code that babel-jest is transpiling (and probably other transpilation mechanisms).

The problem is, I'm trying to access useRouter in a way that will allow me to mock it with Jest. A very common way is like this:

import * as router from 'next/router';

jest.spyOn(router, 'useRouter').mockReturnValue(({
  pathname: '/artist/[artistId]/audience',
  push: pushMockFn,
};

see comment here: #7479 (comment)

This transformer is fundamentally breaking the ability for that to work, and for no ostensible reason.

useRouter is a named export of the module. It should not be hung off of exports.default; it should be directly on the exports object.

[Bonitis]

I am running into a similar problem. As of 12.1.6 all of the tests I have which mock useRouter have broken. I am also not able to compile in dev on that version. I have tested up to 12.1.6-canary.4, which will compile, but still has broken tests. I am trying to include the router fix for memory leaks, which I am experiencing in production. Do I need to change anything about my configuration to get 12.1.6 working after upgrading from 12.1.5? I tested the latest canary build and it has the same problems.

[arackaf]

@Bonitis in a pinch, you can change your test code to

import routerModule from 'next/router';

and then mock routerModule.useRouter but that's really bad. You're basically lying to your tests, which will only happen to work because of this transform.

The only real solution here is for Next to just remove it.

[Bonitis]

@arackaf unfortunately for now I have had to roll back to 12.0.7. The memory leak was what led me here, and it all seems related to the router. I couldn't get any of the latest versions to build properly, so I'm skipping the hack to get my tests to pass until the larger issues get resolved.

[arackaf]

I've created a stand-alone repro: clone this repo, npm i then npm test and you'll see the lone test fail with this error message.

The test file has a commented out line that causes the test to pass, even though it should't.

Edit: link is here:

https://github.com/arackaf/next-repro

[leerob]

@arackaf looks like your link didn't get copy/pasta'd - this one I assume? https://github.com/arackaf/next-repro

[arackaf]

@leerob yes indeed - thank you!

[leerob]

Might be related https://twitter.com/gearsdigital/status/1524852522443325466

[Brooooooklyn]

@arackaf thanks for your repro, I've found the root cause of this issue, it's because the mocked methods on the original module.exports.xxx aren't reflected on the re-exported module.exports.default.xxx. I'm trying to fix it on #36877

@leerob The error thrown in cypress-io/cypress#19382 is ReferenceError require is not defined, it might not be caused by the esm interpolation because it didn't introduce any require statement

[arackaf]

@Brooooooklyn is there no way to just remove this code?

if (typeof exports.default === 'function' || (typeof exports.default === 'object' && exports.default !== null)) {
  Object.assign(exports.default, exports);
  module.exports = exports.default;
}

that was the original cause of the mismatch between what’s exported from the module, and what’s on the actual module.exports object that jest is processing

[Brooooooklyn]

is there no way to just remove this code?

No, these codes are intended to fix #34412 . It's indeed an ugly workaround that should be removed in the future, but I think it's a straightforward way to make type: "module" applications work with next.js at present.

[arackaf]

@Brooooooklyn ohhhh I see. The disastrous Node / esm interop debacle continues. Thanks for the context.

[ijjk]

Hi, this has been updated in v12.1.7-canary.7 of Next.js, please update and give it a try!

[github-actions]

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.