Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic imports testing failure #3345

Closed
1 task done
PrudviGali opened this issue Nov 27, 2017 · 16 comments
Closed
1 task done

Dynamic imports testing failure #3345

PrudviGali opened this issue Nov 27, 2017 · 16 comments

Comments

@PrudviGali
Copy link
Contributor

PrudviGali commented Nov 27, 2017

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

Expected Behavior

Next.js dynamic imports should work with jest test suites.

Current Behavior

It is failing when we run the test with the following error.
TypeError: require.resolveWeak is not a function

Steps to Reproduce (for bugs)

  1. Clone the repo https://github.com/PrudviGali/nextjs-dynamic-import-test-fail
  2. npm install
  3. npm run test

Your Environment

Tech Version
next 4.1.3
node 6.9.1
OS mac OS high sierra
browser NA
etc NA
@OKNoah
Copy link

OKNoah commented Nov 29, 2017

Not sure if related, but the with-dynamic-import example also fails to run.

This module was not found:

* react-dom/lib/ReactReconciler in ./node_modules/next/dist/client/next-dev.js

To install it, you can run: npm install --save react-dom/lib/ReactReconciler
(node:37906) DeprecationWarning: Module.chunks: Use Module.forEachChunk/mapChunks/getNumberOfChunks/isInChunk/addChunk/removeChunk instead
> Ready on http://localhost:3000

@PrudviGali
Copy link
Contributor Author

PrudviGali commented Nov 29, 2017

https://github.com/PrudviGali/nextjs-dynamic-import-test-fail repo is a clone of with-dynamic-import example with jest/enzyme test suites and it is working as expected in the browser. Only problem is with jest testing of dynamic(....).

@Luwangel
Copy link

I've the same issue in my own project using this config.

Tech Version
node 6.9.1
next 4.1.4
react 16.1.1
enzyme 3.2.0
jest 21.2.1
react-test-renderer 16.1.1

@arunoda
Copy link
Contributor

arunoda commented Dec 7, 2017

Here's my comment on this: #3405 (comment)

@arunoda arunoda closed this as completed Dec 7, 2017
@PrudviGali
Copy link
Contributor Author

PrudviGali commented Dec 7, 2017

@arunoda

Thanks for your response, but the issue is not with window object it is with require.resolveWeak and require.ensure methods used in next handle-import.js file. Both these methods are specific to webpack and would not be available in node/commonjs environment. Jest runs in Node environment and does not recognize require.resolveWeak and require.ensure methods.

TC39 dynamic import proposal works pretty well with Jest testing. Please refer to the repo. But Nextjs implementation of dynamic imports fails in jest because of above two methods repo.

Using jsDOM does not help much here. I would request you to re-open this issue and provide a fix if #3405 is not an appropriate way to fix this issue.

@arunoda
Copy link
Contributor

arunoda commented Dec 7, 2017

@PrudviGali what we've in an implementation of the TC39 dynamic proposal with some minor implementation changes to support SSR.

You are trying to do something which is meant to work in the client. (With Jest testing here)
So, I expect it to have proper client env. Otherwise it'll act differently.

@PrudviGali
Copy link
Contributor Author

@arunoda completely agree with you. Using webpack specific methods require.resolveWeak and require.ensure in the dynamic imports pushing it to work only in webpack environments.

We should have a common way to check for non-webpack environments. Using typeof window === 'undefined' is limiting it to Node SSR but not for other environments.

I hope #3414 solves this purpose and detects all non-webpack environments to load fallbacks for require.resolveWeak and require.ensure methods.

@arunoda
Copy link
Contributor

arunoda commented Dec 7, 2017

Let's talk on the other PR.

@PrudviGali
Copy link
Contributor Author

Thanks for merging the pull request @arunoda . #3414 release should fix this issue for everyone.

@cameron-chang
Copy link

Hey, sorry for the necro post, but I'm using Next 5.0.0 and it looks like this issue is still happening. I'm trying to run a jest snapshot test over a page with a dynamically loaded component and running into the TypeError: require.resolveWeak is not a function error.

I followed the thread and found that the fix mentioned above was removed as part of the new universal webpack change, which could have affected the fix. Is this still supposed to work?

e093441#diff-f6a1a602536347313a67de6bccddafd4L19

@brad-decker
Copy link

@arunoda seconded the comment above. Just installed 5.0.0 to test out if that would fix the issue and discovered it does not.

@rileybracken
Copy link

@timneutkens is this supposed to work still with the universal webpack setup?

e093441#diff-f6a1a602536347313a67de6bccddafd4L19

@vladnicula
Copy link
Contributor

handle-imports (e093441#diff-f6a1a602536347313a67de6bccddafd4L19) introduces back the issue with jest.

I tried adding back the check for require.resolveWeak. It allows jest (or node env not using webpack) to run the files that are using dynamic imports. Any reason it was removed @timneutkens?

@vladnicula
Copy link
Contributor

Added back the check for resolve.weakResolve just in case this change was not intentional.

An alternative to this, just for jest, might be something like https://github.com/developit/modify-babel-preset and a jest preprocesor configuration where we use babel-core and a modified configuration of the next preset to exclude the handle import. That might solve this issue for jest without requiring changes in nextjs, but then every other framework that tests components that rely on dynamic loading in next will need to do something similar.

@vladnicula
Copy link
Contributor

Another solution I found which is a bit of a hack is to check for typeof require.resolveWeak !== 'undefined' and only require the module if that is true. This worked for me in my tests as I am importing components via import, not dynamic(import(...)).

For example:

loadGloballyDeferredComponents = () => {
    if (typeof require.resolveWeak === 'undefined') {
      return;
    }
    import('path/to/my/component')
      .then((componentModule) => {
        this.setState({
             MyComponent: componentModule.default,
        });
      })
      .catch((err) => {
        console.error(err);
      });
  }

I'm not too pleased by this because I have to update my code to allow tests to run, and I have to manually include the component that gets set into the state as MyComponent in my test and set it manually.

@vladnicula
Copy link
Contributor

vladnicula commented Apr 27, 2018

Final update on my end. Was able to configure jest to parse all nextjs files that used import('path/to/module') by writing a custom babelrc config for the test environment. This config does not use next/babel, so we never end up using the handle-imports plugin:

{
  "env": {
    "test": {
      "presets": [
        ["env", { "modules": "commonjs" }],
        "react"
      ],
      "plugins": [
        "jest-hoist",
        "inline-dotenv", 
        "transform-inline-environment-variables",
        "transform-object-rest-spread",
        "dynamic-import-node",
        "transform-regenerator"
      ]
    }
  }
}

timneutkens pushed a commit that referenced this issue May 25, 2018
Just in case #3345 was a regression and nextjs should still support non webpack node envs for testing.
lependu pushed a commit to lependu/next.js that referenced this issue Jun 19, 2018
…4208)

Just in case vercel#3345 was a regression and nextjs should still support non webpack node envs for testing.
@lock lock bot locked as resolved and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants