Repository: netlify/zip-it-and-ship-it
Pull request #295 (2021-01-11 13:40:09)

fix: don't fail if critters is missing on Next.js>=10.0.4
**- Summary**

Related to https://github.com/netlify/next-on-netlify/issues/103#issuecomment-755317894
Doesn't fix the original issue as the original issue is about a missing `react-native` module (we don't have a reproduction for it).

This is a result of a [recent change](https://github.com/vercel/next.js/commit/6e4632efe18ea4b65a213f385524d5cc20f25354) in Next.js

**Note:** This use case is not covered by the logic that makes https://github.com/netlify/zip-it-and-ship-it/blob/1e248c6986d43260ac53e18dfa07ab1865589e41/tests/main.js#L127 pass, since we tree shake `Next.js` dependencies instead of copying published files, side files and nested modules.

A better solution would be to support configuring external modules (see https://github.com/netlify/zip-it-and-ship-it/issues/68) so the Next.js plugin (or any other plugin) can generate a relevant bundling configuration based on the framework.
That bundling configuration can change based on the site configuration, for example if the user has `optimizeCss` configured for their Next.js site, we would like to bundle the `critter` dependency.

Regardless, this is an [issue](https://github.com/vercel/next.js/issues/20742) with Next.js as `critters` should be an optional dependency.

**- Test plan**

Added unit tests

**- Description for the changelog**

Don't fail when `critters` module is missing on `Next.js>=10.0.4`

**- A picture of a cute animal (not mandatory but encouraged)**
![image](https://user-images.githubusercontent.com/26760571/104046817-53cb6300-51e9-11eb-97c7-606b8c86d7c8.png)



Patch:
diff --git a/src/node_dependencies/nested.js b/src/node_dependencies/nested.js
index b9438d9..c1e11d7 100644
--- a/src/node_dependencies/nested.js
+++ b/src/node_dependencies/nested.js
@@ -1,3 +1,5 @@
+const { satisfies, validRange } = require('semver')
+
 // Apply the Node.js module logic recursively on its own dependencies, using
 // the `package.json` `dependencies`, `peerDependencies` and
 // `optionalDependencies` keys
@@ -41,7 +43,10 @@ const EXCLUDED_PEER_DEPENDENCIES = new Set(['@prisma/cli', 'prisma2'])
 //  - are not reported when missing
 //  - are included in module dependencies
 const handleModuleNotFound = function ({ error, moduleName, packageJson }) {
-  if (error.code === 'MODULE_NOT_FOUND' && isOptionalModule(moduleName, packageJson)) {
+  if (
+    error.code === 'MODULE_NOT_FOUND' &&
+    (isOptionalModule(moduleName, packageJson) || isExternalCrittersModule(moduleName, packageJson))
+  ) {
     return []
   }
 
@@ -60,4 +65,18 @@ const isOptionalModule = function (
   )
 }
 
+// 'critters' is used only in Next.js >= 10.0.4 when enabling an experimental option and has to be installed manually
+// we ignore it if it's missing
+const isExternalCrittersModule = function (moduleName, { dependencies = {}, devDependencies = {} }) {
+  if (moduleName !== 'critters') {
+    return false
+  }
+  const nextVersion = dependencies.next || devDependencies.next
+  if (!validRange(nextVersion)) {
+    return false
+  }
+  // can the declared Next.js version resolve to 10.0.4 ?
+  return satisfies('10.0.4', nextVersion)
+}
+
 module.exports = { getNestedDependencies, handleModuleNotFound }


Test patch:
diff --git a/tests/fixtures/node-module-next10-critters/function/function.js b/tests/fixtures/node-module-next10-critters/function/function.js
new file mode 100644
index 0000000..41d8840
--- /dev/null
+++ b/tests/fixtures/node-module-next10-critters/function/function.js
@@ -0,0 +1,4 @@
+// eslint-disable-next-line import/no-unassigned-import
+require('./renderNextPage')
+
+module.exports = true
diff --git a/tests/fixtures/node-module-next10-critters/function/renderNextPage.js b/tests/fixtures/node-module-next10-critters/function/renderNextPage.js
new file mode 100644
index 0000000..7fd8d6e
--- /dev/null
+++ b/tests/fixtures/node-module-next10-critters/function/renderNextPage.js
@@ -0,0 +1,6 @@
+/* eslint-disable unicorn/filename-case */
+// eslint-disable-next-line import/no-unassigned-import
+require('next/dist/next-server/server/render.js')
+
+module.exports = true
+/* eslint-enable unicorn/filename-case */
diff --git a/tests/fixtures/node-module-next10-critters/node_modules/next/dist/next-server/server/render.js b/tests/fixtures/node-module-next10-critters/node_modules/next/dist/next-server/server/render.js
new file mode 100644
index 0000000..6480be1
--- /dev/null
+++ b/tests/fixtures/node-module-next10-critters/node_modules/next/dist/next-server/server/render.js
@@ -0,0 +1,8 @@
+const render = () => {
+  if (renderOpts.optimizeCss) {
+    // eslint-disable-next-line node/no-extraneous-require
+    require('critters')
+  }
+}
+
+module.exports = render
diff --git a/tests/fixtures/node-module-next10-critters/node_modules/next/index.js b/tests/fixtures/node-module-next10-critters/node_modules/next/index.js
new file mode 100644
index 0000000..33c1891
--- /dev/null
+++ b/tests/fixtures/node-module-next10-critters/node_modules/next/index.js
@@ -0,0 +1,1 @@
+module.exports = true
diff --git a/tests/fixtures/node-module-next10-critters/node_modules/next/package.json b/tests/fixtures/node-module-next10-critters/node_modules/next/package.json
new file mode 100644
index 0000000..0967ef4
--- /dev/null
+++ b/tests/fixtures/node-module-next10-critters/node_modules/next/package.json
@@ -0,0 +1,1 @@
+{}
diff --git a/tests/fixtures/node-module-next10-critters/package.json b/tests/fixtures/node-module-next10-critters/package.json
new file mode 100644
index 0000000..7d26412
--- /dev/null
+++ b/tests/fixtures/node-module-next10-critters/package.json
@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "next": "^10.0.0"
+  }
+}
diff --git a/tests/fixtures/node-module-next9-critters/function.js b/tests/fixtures/node-module-next9-critters/function.js
new file mode 100644
index 0000000..fced3c7
--- /dev/null
+++ b/tests/fixtures/node-module-next9-critters/function.js
@@ -0,0 +1,6 @@
+try {
+  // eslint-disable-next-line import/no-unresolved, import/no-unassigned-import, node/global-require
+  require('critters')
+} catch (error) {}
+
+module.exports = true
diff --git a/tests/fixtures/node-module-next9-critters/package.json b/tests/fixtures/node-module-next9-critters/package.json
new file mode 100644
index 0000000..6b24a08
--- /dev/null
+++ b/tests/fixtures/node-module-next9-critters/package.json
@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "next": "^9.5.5"
+  }
+}
diff --git a/tests/main.js b/tests/main.js
index 938fee8..a219fb4 100644
--- a/tests/main.js
+++ b/tests/main.js
@@ -139,6 +139,14 @@ test('Throws on missing non-optional peer dependencies', async (t) => {
   await t.throwsAsync(zipNode(t, 'node-module-peer-not-optional'))
 })
 
+test('Throws on missing critters dependency for Next.js 9', async (t) => {
+  await t.throwsAsync(zipNode(t, 'node-module-next9-critters'))
+})
+
+test('Ignore missing critters dependency for Next.js 10', async (t) => {
+  await zipNode(t, 'node-module-next10-critters')
+})
+
 // We persist `package.json` as `package.json.txt` in git. Otherwise ESLint
 // tries to load when linting sibling JavaScript files. In this test, we
 // temporarily rename it to an actual `package.json`.