Repository: netlify/zip-it-and-ship-it
Pull request #242 (2020-11-19 14:10:42)

feat(js): add list of tree shakable external dependencies
**- Summary**

Still in draft as it needs testing (automated and manual) and some code cleanup.
For some reason I'm getting 500 errors when trying to deploy functions locally so I'm having a hard time trying to test this.

**- Test plan**

Going to add tests once I'm finished with verifying this locally.

**- Description for the changelog**

Add a list of tree shakable external modules to help reduce bundle size.
I'm only considering adding an escape hatch for this via a env variable (or any other configuration) so this behavior can be disabled/changed.

**- A picture of a cute animal (not mandatory but encouraged)**

🐱 

erezrokah: > For some reason I'm getting 500 errors when trying to deploy functions locally so I'm having a hard time trying to test this.

Looks like I'm hitting the issue described in [this thread](https://community.netlify.com/t/500-internal-server-error-on-cdn-diffing-files/20715/23)

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e816ae0..73f279b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,7 +7,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
 
-## [v1.6.1](https://github.com/netlify/zip-it-and-ship-it/compare/v1.6.0...v1.6.1)
+## [v1.6.2](https://github.com/netlify/zip-it-and-ship-it/compare/v1.6.1...v1.6.2)
+
+### Merged
+
+- Rename variables [`#264`](https://github.com/netlify/zip-it-and-ship-it/pull/264)
+- chore: add commit linting [`#265`](https://github.com/netlify/zip-it-and-ship-it/pull/265)
+- Use `Set` in Node.js dependencies logic [`#263`](https://github.com/netlify/zip-it-and-ship-it/pull/263)
+- Add `.vscode` to `.gitignore` [`#262`](https://github.com/netlify/zip-it-and-ship-it/pull/262)
+
+## [v1.6.1](https://github.com/netlify/zip-it-and-ship-it/compare/v1.6.0...v1.6.1) - 2020-12-14
 
 ### Merged
 
diff --git a/package-lock.json b/package-lock.json
index efd67b7..092d8c9 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "@netlify/zip-it-and-ship-it",
-  "version": "1.6.1",
+  "version": "1.6.2",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
diff --git a/package.json b/package.json
index c7b92f8..7f1b075 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@netlify/zip-it-and-ship-it",
-  "version": "1.6.1",
+  "version": "1.6.2",
   "description": "Zip it and ship it",
   "main": "./src/main.js",
   "bin": {
diff --git a/src/dependencies.js b/src/dependencies.js
index da392de..15358a4 100644
--- a/src/dependencies.js
+++ b/src/dependencies.js
@@ -70,7 +70,7 @@ const getPackageJson = function(packageRoot) {
   }
 }
 
-const getFileDependencies = async function({ path, packageJson, state }) {
+const getFileDependencies = async function({ path, packageJson, state, treeShakeNext }) {
   if (state.localFiles.has(path)) {
     return []
   }
@@ -84,25 +84,43 @@ const getFileDependencies = async function({ path, packageJson, state }) {
   const dependencies = precinct.paperwork(path, { includeCore: false })
 
   const depsPaths = await Promise.all(
-    dependencies.map(dependency => getImportDependencies({ dependency, basedir, packageJson, state }))
+    dependencies.map(dependency => getImportDependencies({ dependency, basedir, packageJson, state, treeShakeNext }))
   )
   return [].concat(...depsPaths)
 }
 
-const getImportDependencies = function({ dependency, basedir, packageJson, state }) {
-  if (LOCAL_IMPORT_REGEXP.test(dependency)) {
-    return getTreeShakedDependencies({ dependency, basedir, packageJson, state })
+const getImportDependencies = function({ dependency, basedir, packageJson, state, treeShakeNext }) {
+  const shouldTreeShakeNext = treeShakeNext || isNextOnNetlify(dependency)
+  if (shouldTreeShake(dependency, shouldTreeShakeNext)) {
+    return getTreeShakedDependencies({ dependency, basedir, packageJson, state, treeShakeNext: shouldTreeShakeNext })
   }
 
   return getAllDependencies({ dependency, basedir, state, packageJson })
 }
 
+const isNextOnNetlify = function(dependency) {
+  return basename(dependency, '.js') === 'renderNextPage'
+}
+
+// we tree shake direct local requires to bundle only relevant code
+// we don't tree shake external dependencies since they can break our tree shaking logic by:
+// 1. doing dynamic requires (e.g. require(dynamicPath)
+// 2. reading files (e.g. fs.readFileSync(someNonJsFile)
+// an exception is when the function was generated by `next-on-netlify` to avoid bundling the entire `Next.js` framework
+const shouldTreeShake = function(dependency, treeShakeNext) {
+  if (LOCAL_IMPORT_REGEXP.test(dependency)) {
+    return true
+  }
+
+  return treeShakeNext && getModuleName(dependency) === 'next'
+}
+
 const LOCAL_IMPORT_REGEXP = /^(\.|\/)/
 
 // When a file requires another one, we apply the top-level logic recursively
-const getTreeShakedDependencies = async function({ dependency, basedir, packageJson, state }) {
+const getTreeShakedDependencies = async function({ dependency, basedir, packageJson, state, treeShakeNext }) {
   const path = await resolvePathPreserveSymlinks(dependency, basedir)
-  const depsPath = await getFileDependencies({ path, packageJson, state })
+  const depsPath = await getFileDependencies({ path, packageJson, state, treeShakeNext })
   return [path, ...depsPath]
 }
 
diff --git a/src/fixtures/node-module-next-on-netlify/function/function.js b/src/fixtures/node-module-next-on-netlify/function/function.js
new file mode 100644
index 0000000..4ad0ece
--- /dev/null
+++ b/src/fixtures/node-module-next-on-netlify/function/function.js
@@ -0,0 +1,3 @@
+require('./renderNextPage')
+
+module.exports = true
diff --git a/src/fixtures/node-module-next-on-netlify/function/renderNextPage.js b/src/fixtures/node-module-next-on-netlify/function/renderNextPage.js
new file mode 100644
index 0000000..9f565f3
--- /dev/null
+++ b/src/fixtures/node-module-next-on-netlify/function/renderNextPage.js
@@ -0,0 +1,4 @@
+// eslint-disable-next-line node/no-extraneous-require
+require('next/dist/next-server/lib/constants')
+
+module.exports = true
diff --git a/src/fixtures/node-module-next-on-netlify/node_modules/next/dist/compiled/semver.js b/src/fixtures/node-module-next-on-netlify/node_modules/next/dist/compiled/semver.js
new file mode 100644
index 0000000..33c1891
--- /dev/null
+++ b/src/fixtures/node-module-next-on-netlify/node_modules/next/dist/compiled/semver.js
@@ -0,0 +1,1 @@
+module.exports = true
diff --git a/src/fixtures/node-module-next-on-netlify/node_modules/next/dist/next-server/lib/constants.js b/src/fixtures/node-module-next-on-netlify/node_modules/next/dist/next-server/lib/constants.js
new file mode 100644
index 0000000..34dc59c
--- /dev/null
+++ b/src/fixtures/node-module-next-on-netlify/node_modules/next/dist/next-server/lib/constants.js
@@ -0,0 +1,6 @@
+// eslint-disable-next-line node/no-extraneous-require
+require('next/dist/compiled/semver')
+
+module.exports = {
+  NAME: 'Value'
+}
diff --git a/src/fixtures/node-module-next-on-netlify/node_modules/next/dist/other.js b/src/fixtures/node-module-next-on-netlify/node_modules/next/dist/other.js
new file mode 100644
index 0000000..fa8f890
--- /dev/null
+++ b/src/fixtures/node-module-next-on-netlify/node_modules/next/dist/other.js
@@ -0,0 +1,4 @@
+
+module.exports = () => {
+  console.log('other')
+}
diff --git a/src/fixtures/node-module-next-on-netlify/node_modules/next/index.js b/src/fixtures/node-module-next-on-netlify/node_modules/next/index.js
new file mode 100644
index 0000000..33c1891
--- /dev/null
+++ b/src/fixtures/node-module-next-on-netlify/node_modules/next/index.js
@@ -0,0 +1,1 @@
+module.exports = true
diff --git a/src/fixtures/node-module-next-on-netlify/node_modules/next/package.json b/src/fixtures/node-module-next-on-netlify/node_modules/next/package.json
new file mode 100644
index 0000000..0967ef4
--- /dev/null
+++ b/src/fixtures/node-module-next-on-netlify/node_modules/next/package.json
@@ -0,0 +1,1 @@
+{}
diff --git a/src/fixtures/node-module-next/function.js b/src/fixtures/node-module-next/function.js
new file mode 100644
index 0000000..9f565f3
--- /dev/null
+++ b/src/fixtures/node-module-next/function.js
@@ -0,0 +1,4 @@
+// eslint-disable-next-line node/no-extraneous-require
+require('next/dist/next-server/lib/constants')
+
+module.exports = true
diff --git a/src/fixtures/node-module-next/node_modules/next/dist/compiled/semver.js b/src/fixtures/node-module-next/node_modules/next/dist/compiled/semver.js
new file mode 100644
index 0000000..33c1891
--- /dev/null
+++ b/src/fixtures/node-module-next/node_modules/next/dist/compiled/semver.js
@@ -0,0 +1,1 @@
+module.exports = true
diff --git a/src/fixtures/node-module-next/node_modules/next/dist/next-server/lib/constants.js b/src/fixtures/node-module-next/node_modules/next/dist/next-server/lib/constants.js
new file mode 100644
index 0000000..34dc59c
--- /dev/null
+++ b/src/fixtures/node-module-next/node_modules/next/dist/next-server/lib/constants.js
@@ -0,0 +1,6 @@
+// eslint-disable-next-line node/no-extraneous-require
+require('next/dist/compiled/semver')
+
+module.exports = {
+  NAME: 'Value'
+}
diff --git a/src/fixtures/node-module-next/node_modules/next/dist/other.js b/src/fixtures/node-module-next/node_modules/next/dist/other.js
new file mode 100644
index 0000000..fa8f890
--- /dev/null
+++ b/src/fixtures/node-module-next/node_modules/next/dist/other.js
@@ -0,0 +1,4 @@
+
+module.exports = () => {
+  console.log('other')
+}
diff --git a/src/fixtures/node-module-next/node_modules/next/index.js b/src/fixtures/node-module-next/node_modules/next/index.js
new file mode 100644
index 0000000..33c1891
--- /dev/null
+++ b/src/fixtures/node-module-next/node_modules/next/index.js
@@ -0,0 +1,1 @@
+module.exports = true
diff --git a/src/fixtures/node-module-next/node_modules/next/package.json b/src/fixtures/node-module-next/node_modules/next/package.json
new file mode 100644
index 0000000..0967ef4
--- /dev/null
+++ b/src/fixtures/node-module-next/node_modules/next/package.json
@@ -0,0 +1,1 @@
+{}


Test patch:
diff --git a/src/main.test.js b/src/main.test.js
index d5e09df..e5ca6b3 100644
--- a/src/main.test.js
+++ b/src/main.test.js
@@ -71,6 +71,34 @@ test('Include most files from node modules', async t => {
   t.true(htmlExists)
 })
 
+test('Includes specific Next.js dependencies when using next-on-netlify', async t => {
+  const { tmpDir } = await zipNode(t, 'node-module-next-on-netlify')
+  const [constantsExists, semverExists, otherExists, indexExists] = await Promise.all([
+    pathExists(`${tmpDir}/src/node_modules/next/dist/next-server/lib/constants.js`),
+    pathExists(`${tmpDir}/src/node_modules/next/dist/compiled/semver.js`),
+    pathExists(`${tmpDir}/src/node_modules/next/dist/other.js`),
+    pathExists(`${tmpDir}/src/node_modules/next/index.js`)
+  ])
+  t.true(constantsExists)
+  t.true(semverExists)
+  t.false(otherExists)
+  t.false(indexExists)
+})
+
+test('Includes all Next.js dependencies when not using next-on-netlify', async t => {
+  const { tmpDir } = await zipNode(t, 'node-module-next')
+  const [constantsExists, semverExists, otherExists, indexExists] = await Promise.all([
+    pathExists(`${tmpDir}/src/node_modules/next/dist/next-server/lib/constants.js`),
+    pathExists(`${tmpDir}/src/node_modules/next/dist/compiled/semver.js`),
+    pathExists(`${tmpDir}/src/node_modules/next/dist/other.js`),
+    pathExists(`${tmpDir}/src/node_modules/next/index.js`)
+  ])
+  t.true(constantsExists)
+  t.true(semverExists)
+  t.true(otherExists)
+  t.true(indexExists)
+})
+
 test('Throws on runtime errors', async t => {
   await t.throwsAsync(zipNode(t, 'node-module-error'))
 })