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

Serverless build requires missing critters dependency, which causes netlify deploy to fail #20742

Closed
frigus02 opened this issue Jan 4, 2021 · 5 comments
Labels
bug Issue was opened via the bug report template.

Comments

@frigus02
Copy link

frigus02 commented Jan 4, 2021

What version of Next.js are you using?

10.0.4

What version of Node.js are you using?

15.5.0

What browser are you using?

any

What operating system are you using?

macOS, Linux

How are you deploying your application?

next build && next-on-netlify && netlify deploy

Describe the Bug

We deploy a Next.js application using the serverless target to Netlify. It worked on 10.0.3, but fails on 10.0.4.

The netlify deploy command errors with:

$ /node_modules/.bin/netlify deploy -d out_publish -f out_functions --prod
Deploy path:        /out_publish
Functions path:     /out_functions
Configuration path: /netlify.toml
Deploying to main site URL...
- Hashing files...
 ›   Warning: 
 ›   {
 ›     "code": "MODULE_NOT_FOUND",
 ›     "requireStack": [...]
 ›   }
 ›
    Error: In file "/out_functions/next_advancedsearch/next_advancedsearch.js": Cannot find module 'critters'
    Require stack:
    - /node_modules/@netlify/zip-it-and-ship-it/src/resolve.js
    - ...
    /node_modules/netlify-cli/bin/run
    Code: MODULE_NOT_FOUND

It seems the issue is that a require("critters") is added to the next build serverless output. We can see that here:

$ rg critters .next/
...
.next/serverless/pages/advanced-search.js
52206:module.exports = require("critters");
...

We assume this is because the following code is not removed by the JS minifier:

if (renderOpts.optimizeCss) {
// eslint-disable-next-line import/no-extraneous-dependencies
const Critters = require('critters')

We tried to change this to use the environment variable which caused the code to be removed on the next build. But we haven't tested if the optimizeCss feature still works 🙂.

  if (process.env.__NEXT_OPTIMIZE_CSS) {

The issue has also been reported here: https://community.netlify.com/t/issue-while-deploying-next-app/28861. As mentioned installing "critters" as a dev dependency is another workaround.

Our understanding is that this is an issue of Next.js. Please let us know if we should report this on the Netlify CLI instead.

Expected Behavior

The next build output does not include critters if the optimizeCss feature is not enabled.

To Reproduce

Create example app:

yarn create next-app

Change build target to serverless:

// next.config.js
module.exports = {
  target: "serverless"
};

Build and find critters in the output:

next build
rg critters .next/

To reproduce the Netlify error install next-on-netlify and netlify-cli and attempt to deploy the application:

next-on-netlify
netlify deploy -d out_publish -f out_functions
@frigus02 frigus02 added the bug Issue was opened via the bug report template. label Jan 4, 2021
@frigus02 frigus02 changed the title Deployment on Netlify errors with Cannot find module 'critters'" Serverless build requires missing critters dependency, which causes netlify deploy to fail Jan 4, 2021
ahmad-reza619 added a commit to ahmad-reza619/site that referenced this issue Jan 9, 2021
ahmad-reza619 added a commit to ahmad-reza619/site that referenced this issue Jan 9, 2021
@erezrokah
Copy link

Hello 👋 Erez from Netlify here.
We've published a fix for this issue to our CLI and build environment.

TLDR for the cause: A recent change added critters as a dependency without including it as an prod/dev/optional/peer dependency which makes our bundling logic very unhappy.

See here and here for reasons not to include it.

We have an active discussion on how to avoid such cases in the future.

This issue was also reported here

@frigus02
Copy link
Author

Thanks @erezrokah. I can confirm that your fix works for us.

Do I understand it correctly, that critters should probably be a some kind of dependency in the next package? So this issue here is still valid?

@erezrokah
Copy link

Glad to hear that @frigus02 and thank you for confirming.

So this issue here is still valid?

Yes I think the issue is still valid. Any bundler would probably run into this issue.

@timneutkens
Copy link
Member

Going to close this given that the particular critters require is optional (part of an experimental feature) and as such in an if() statement that the hosting platform you're using does not support (now fixed it seems).

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests

4 participants