vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.95k stars 26.87k forks source link

Cannot access ... before initialization #39985

Open PepijnSenders opened 2 years ago

PepijnSenders commented 2 years ago

Verify canary release

Provide environment information

yarn next info

    Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 21.6.0: Sat Jun 18 17:07:22 PDT 2022; root:xnu-8020.140.41~1/RELEASE_ARM64_T6000
    Binaries:
      Node: 16.16.0
      npm: 8.11.0
      Yarn: 3.2.0
      pnpm: N/A
    Relevant packages:
      next: 12.2.4
      eslint-config-next: 12.2.0
      react: 17.0.2
      react-dom: 17.0.2

warn  - Latest canary version not detected, detected: "12.2.4", newest: "12.2.6-canary.5".
        Please try the latest canary version (`npm install next@canary`) to confirm the issue still exists before creating a new issue.
        Read more - https://nextjs.org/docs/messages/opening-an-issue

What browser are you using? (if relevant)

Chrome

How are you deploying your application? (if relevant)

yarn jest

Describe the Bug

Between 12.2.0 and 12.2.1 a big chunk of our unit tests broke. Any test using jest.requireActual('') to not mock some of the modules fails with an error like this:

    ReferenceError: Cannot access 'buildError' before initialization

      81 |
    > 82 | export const buildError = {

I tried to debug this, but sadly am not able to view the swc compiled source files.

Additionally all tests using jest.mock('next/router') have to be changed to:

jest.mock('next/router', () => ({
  useRouter: jest.fn(),
});

Because that now yielded the following error:

No router instance found.
    You should only use "next/router" on the client side of your app.

Expected Behavior

jest.requireActual('') to keep working.

Link to reproduction

Tried to reproduce in simple repo but wasn't able to get the same behaviour

To Reproduce

This part of the test that breaks:

jest.mock('./bla', () => ({
  ...jest.requireActual('./bla'),
  useBla: jest.fn(),
});

describe('SomeComponent', () => {
  const getComponent = (): ShallowWrapper => shallow(<SomeComponent />);

  it('should bla', () => {
    const component = getComponent();

    expect(component.html()).toBeNull();
  });
});
export const SomeComponent: React.FC = () => {
  const bla = useBla();

  return bla;
};
export const buildError = {
  build: () => 'test',
};

export const useBla = () => 'bla';

Compiler options:

    compiler: {
      styledComponents: {
        cssProp: false,
      },

      emotion:
        // Don't compile with jest otherwise all emotion components become EmotionCSSPropInternal
        phase === PHASE_TEST
          ? null
          : {
              sourceMap: true,
              autoLabel: 'dev-only',
              labelFormat: 'local',
            },

      removeConsole:
        phase === PHASE_PRODUCTION_BUILD || phase === PHASE_PRODUCTION_SERVER,
    },
balazsorban44 commented 2 years ago

Hi, have you seen this warning?:

warn - Latest canary version not detected, detected: "12.2.4", newest: "12.2.6-canary.5".

It would be nice to verify if this has not yet been fixed.

If your repo is private, could you send me an invite so I can check it out more closely? Otherwise a minimal reproduction could help us find the issue more quickly.

PepijnSenders commented 2 years ago

Hey @balazsorban44, I tried with canary as well, and it failed the same way. Our repo is massive and you'd probably need to sign an NDA etc to gain access. Do you maybe know if something changed regarding jest/swc compilation between 12.2.0 and 12.2.1 that could have broken this?

balazsorban44 commented 2 years ago

Adding a minimal reproduction would be desirable then, that we could use to add a test to avoid regressions if we can find a bug.

Trying to pinpoint the exact canary release could help investigate this further. As of writing, 12.2.0-12.2.1 canary releases are on this release page: https://github.com/vercel/next.js/releases?page=6

If we have the exact version, we can find the commit and get a better understanding of the issue.

PepijnSenders commented 2 years ago

This is a simple reprodruction of the next/router issue. The other one will be a bit more complex cause I feel like it only happens in deeper import trees.

PepijnSenders commented 2 years ago

@balazsorban44 that's smart, I'll see which canary it broke with!

PepijnSenders commented 2 years ago

@balazsorban44 this release broke it: https://github.com/vercel/next.js/releases/tag/v12.2.3-canary.1. 12.2.3-canary.0 still worked.

PepijnSenders commented 2 years ago

I created a resolution for @swc/helpers and terser, which were upgraded here: https://github.com/vercel/next.js/pull/38427/files#diff-9c1a3867443c54525b4f24ef171f231a6e8bb065ffc8b7b62c4843d5ff62dd42L75. Didn't fix the issue. My bet is the swc upgrade: https://github.com/vercel/next.js/pull/38347. Not sure how to revert this change though.

PepijnSenders commented 2 years ago

I made my app force-download the wasm binary and hardcoded the version to canary-1 here: https://github.com/vercel/next.js/blob/17244b8a2340be2569338c55f8c03cd697131365/packages/next/lib/download-wasm-swc.ts#L18, whilst being on canary-0. That breaks the test. So it's definitely an upgrade on the SWC WASM binary, but as I see there's a lot of changes in there 😅

PepijnSenders commented 2 years ago

I compiled files with latest @swc/core and @swc/core@1.2.188. And the main difference is this:

Added in latest:

function ownKeys(object, enumerableOnly) {  
    var keys = Object.keys(object); 
    if (Object.getOwnPropertySymbols) { 
        var symbols = Object.getOwnPropertySymbols(object); 
        if (enumerableOnly) {   
            symbols = symbols.filter(function(sym) {    
                return Object.getOwnPropertyDescriptor(object, sym).enumerable; 
            }); 
        }   
        keys.push.apply(keys, symbols); 
    }   
    return keys;    
}   
function _objectSpreadProps(target, source) {   
    source = source != null ? source : {};  
    if (Object.getOwnPropertyDescriptors) { 
        Object.defineProperties(target, Object.getOwnPropertyDescriptors(source));  
    } else {    
        ownKeys(Object(source)).forEach(function(key) { 
            Object.defineProperty(target, key, Object.getOwnPropertyDescriptor(source, key));   
        }); 
    }   
    return target;  
}

Changed in latest:

jest.mock("react-redux-7", function() { 
    return _objectSpreadProps(_objectSpread({}, jest.requireActual("react-redux-7")), { 
        useSelector: function(selector) {   
            return selector();  
        }   
    }); 
});

Original:

jest.mock("react-redux-7", function() {
    return _objectSpread({}, jest.requireActual("react-redux-7"), {
        useSelector: function(selector) {
            return selector();
        }
    });
});

So a quick guess from this finding is that _objectSpreadProps is messing stuff up.

I don't think I can do much more here, let me know if you need more info @balazsorban44 !

LarsEjaas commented 2 years ago

I am unsure if the issue I am seeing is 100% related, but I am having a lot of unit tests breaking also currently after upgrading Jest and Nextjs.

My issue is mainly related to the Jest.spyOn method. I get the error: TypeError: Cannot redefine property: default. This looks like it is related to SWC recently changed ESM behavior, so exports now are immutable - please see this issue: https://github.com/swc-project/swc/discussions/5151#discussioncomment-3149154

But I see there is a plugin: https://github.com/magic-akari/jest_workaround anyone tried this approach with Next.js?

Sorry if this is unrelated - just couldn't help thinking this may be related when you are referring to jest.requireActual('') failing.

PepijnSenders commented 2 years ago

@LarsEjaas, definitely looks related. Tried this plugin but running into some issues: https://github.com/magic-akari/jest_workaround/issues/10#issue-1366663897.

PepijnSenders commented 2 years ago

@balazsorban44 any idea on this issue?

maccman commented 1 year ago

Just ran into this...