Repository: netlify/zip-it-and-ship-it
Pull request #681 (2021-09-27 15:57:33)

fix: force esbuild target in zisi
**- Summary**

When parsing files with esbuild to find dependencies, we rely on the default `target` property set by esbuild, which is `esnext`. However, this is overridden by any `compilerOptions.target` property set in a `tsconfig.json`. When this value is set to `es5`, parsing with esbuild can fail because some expressions can't be lowered to ES5.

This PR explicitly sets `target: "esnext"` in esbuild so that no transpiling is required.

**- Test plan**

Added a new test.

**- A picture of a cute animal (not mandatory but encouraged)**

![f03a349219e07c262f961a9afefb9a66](https://user-images.githubusercontent.com/4162329/134943242-ed567194-67a1-4340-82bc-eaff0e5b846a.jpg)




github-actions[bot]: ## ⏱ Benchmark results

*Comparing with 12a08ba7857d59a07954f296d3c4993460c49f80*

### largeDepsZisi: 1m 11.3s
 ⬇️ **0.37% decrease** vs. 12a08ba7857d59a07954f296d3c4993460c49f80
```
^                                                                 1m 13.5s1m 14.3s                        
│                                 1m 10.3s        1m 11.6s          ┌──┐    ┌──┐                  1m 11.3s
│                         1m 7.4s   ┌──┐   1m 8s    ┌──┐            |  |    |  |                    ┌──┐  
│                           ┌──┐    |  |    ┌──┐    |  |            |  |    |  |                    |▒▒|  
│ ─57.6s────58s─────────────┼──┼────┼──┼────┼──┼────┼──┼────────────┼──┼────┼──┼────────────────────|▒▒|──
│   ┌──┐    ┌──┐   56.5s    |  |    |  |    |  |    |  |            |  |    |  |   57.6s            |▒▒|  
│   |  |    |  |    ┌──┐    |  |    |  |    |  |    |  |   52.9s    |  |    |  |    ┌──┐   50.8s    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐    |  |    |  |    |  |    ┌──┐    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
```
<details>
<summary>Legend</summary>

- T-30 (12a08ba7857d59a07954f296d3c4993460c49f80): 1m 11.6s
- T-29 (ce6d1ce74527981b97264afa32884e7345f482fc): 1m 14.1s
- T-28 (d646be2424960c4dea546971ba255e4de8d345a8): 1m 10.4s
- T-27 (2de54262c78ac5078421630a5d5edf177c2822ac): 1m 9.5s
- T-26 (f756b03135e2300f0f3ba679466c7ad51b68f6c7): 1m 7.3s
- T-25 (5bf51d0b4938dcff42f5acefe309a67e9651d89b): 1m 5.8s
- T-24 (c28eaafe7288d95822cffd2dd6d71c11ba420740): 1m 1.9s
- T-23 (b843aae745fb9b3c3c9e7c8cf0fd328cf48587ef): 56.7s
- T-22 (cdbcad2c916610e2d12d601990bd67d1be18bc04): 1m 9s
- T-21 (ff051cd9dd9994117e12ee2f9899dc1df960fa4f): 1m 2.2s
- T-20 (c649c299f7857d8d40ea6fdb9fe7d5e13461c5e1): 1m 4.8s
- T-19 (9c628c8b1667a5714c33f1f178aa105beb7a755a): 57.8s
- T-18 (bf6a59bbe4911b6969f84b8b9520c5661a6a8aab): 1m 10.4s
- T-17 (85cc47d0677505ca2b6f2cfdf4662fb9c1f9a4b9): 1m 10.1s
- T-16 (b105692e48b8ff2c175a7f93e9e28b053c8b5453): 1m 7.2s
- T-15 (9b0c6d56e4e05704122fdd55cacc81d75cf40aa1): 58s
- T-14 (53d0281332b686165f2f3660b2c51efff5fea2a2): 1m 5.7s
- T-13 (b468b2abe161bdd1e330b0f73bede56b497565cf): 59.1s
- T-12 (a9b86e2f9de9b0929dda849afee625651b4dc7cb): 57.6s
- T-11 (4c64157467b1e5186bb66a7af8cca71c2b8ad633): 58s
- T-10 (fbcb80f1a13b5c633331a51f5a8fdb00cebad0d8): 56.5s
- T-9 (cde6c632dff6ba227cf1b3017a1c6e0dced82c11): 1m 7.4s
- T-8 (c1e762d844569d87cf049a04999e9efeb1ece034): 1m 10.3s
- T-7 (cc489697c9efa5994b187ba4a3271d2d3ef89ad4): 1m 8s
- T-6 (ce3c162cbbca918748763b66df2e29065bb41e47): 1m 11.6s
- T-5 (fc6b3efdc887db8ed20bffa594cc953085ade70e): 52.9s
- T-4 (cde4e512323f9859ab4db145f30b091984cdaad4): 1m 13.5s
- T-3 (cde22957f16be7748d3090f72da98d74a0725f1e): 1m 14.3s
- T-2 (e463691e3e5b289ee7adc941fb8f7c6974effa31): 57.6s
- T-1 (ecb8856c139b7aa45aa866050cb0559e1e30ee37): 50.8s
- **T (current commit): 1m 11.3s**</details>
<!--delta:benchmark@{}-->

Patch:
diff --git a/src/runtimes/node/list_imports.js b/src/runtimes/node/list_imports.js
index 8ea4fe0..f67b4a8 100644
--- a/src/runtimes/node/list_imports.js
+++ b/src/runtimes/node/list_imports.js
@@ -4,6 +4,11 @@ const { tmpName } = require('tmp-promise')
 
 const { safeUnlink } = require('../../utils/fs')
 
+// Maximum number of log messages that an esbuild instance will produce. This
+// limit is important to avoid out-of-memory errors due to too much data being
+// sent in the Go<>Node IPC channel.
+const ESBUILD_LOG_LIMIT = 10
+
 const getListImportsPlugin = ({ imports, path }) => ({
   name: 'list-imports',
   setup(build) {
@@ -35,11 +40,14 @@ const listImports = async ({ path }) => {
 
   try {
     await esbuild.build({
-      entryPoints: [path],
       bundle: true,
+      entryPoints: [path],
+      logLevel: 'error',
+      logLimit: ESBUILD_LOG_LIMIT,
       outfile: targetPath,
       platform: 'node',
       plugins: [getListImportsPlugin({ imports, path })],
+      target: 'esnext',
     })
   } finally {
     await safeUnlink(targetPath)


Test patch:
diff --git a/tests/fixtures/local-node-module-destructure-require/function.js b/tests/fixtures/local-node-module-destructure-require/function.js
new file mode 100644
index 0000000..c9a2156
--- /dev/null
+++ b/tests/fixtures/local-node-module-destructure-require/function.js
@@ -0,0 +1,3 @@
+const { stack } = require('@netlify/mock-package/stack')
+
+module.exports = { stack }
diff --git a/tests/fixtures/local-node-module-destructure-require/node_modules/@netlify/mock-package/index.js b/tests/fixtures/local-node-module-destructure-require/node_modules/@netlify/mock-package/index.js
new file mode 100644
index 0000000..dc03a1a
--- /dev/null
+++ b/tests/fixtures/local-node-module-destructure-require/node_modules/@netlify/mock-package/index.js
@@ -0,0 +1,3 @@
+const stack = require('./stack')
+
+module.exports = { stack }
diff --git a/tests/fixtures/local-node-module-destructure-require/node_modules/@netlify/mock-package/package.json b/tests/fixtures/local-node-module-destructure-require/node_modules/@netlify/mock-package/package.json
new file mode 100644
index 0000000..970c87d
--- /dev/null
+++ b/tests/fixtures/local-node-module-destructure-require/node_modules/@netlify/mock-package/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "@netlify/mock-package",
+  "version": "1.0.0"
+}
diff --git a/tests/fixtures/local-node-module-destructure-require/node_modules/@netlify/mock-package/stack.js b/tests/fixtures/local-node-module-destructure-require/node_modules/@netlify/mock-package/stack.js
new file mode 100644
index 0000000..5c06561
--- /dev/null
+++ b/tests/fixtures/local-node-module-destructure-require/node_modules/@netlify/mock-package/stack.js
@@ -0,0 +1,1 @@
+module.exports = 'jam'
diff --git a/tests/fixtures/local-node-module-destructure-require/package.json b/tests/fixtures/local-node-module-destructure-require/package.json
new file mode 100644
index 0000000..4f36436
--- /dev/null
+++ b/tests/fixtures/local-node-module-destructure-require/package.json
@@ -0,0 +1,7 @@
+{
+  "name": "local-node-module-destructure-require",
+  "version": "1.0.0",
+  "dependencies": {
+    "@netlify/mock-package": "1.0.0"
+  }
+}
diff --git a/tests/fixtures/local-node-module-destructure-require/tsconfig.json b/tests/fixtures/local-node-module-destructure-require/tsconfig.json
new file mode 100644
index 0000000..42e12d9
--- /dev/null
+++ b/tests/fixtures/local-node-module-destructure-require/tsconfig.json
@@ -0,0 +1,5 @@
+{
+  "compilerOptions": {
+    "target": "ES5"
+  }
+}
diff --git a/tests/main.js b/tests/main.js
index 7725a08..779bf10 100644
--- a/tests/main.js
+++ b/tests/main.js
@@ -173,6 +173,21 @@ testBundlers('Can require deep paths in node modules', [ESBUILD, ESBUILD_ZISI, D
   }
 })
 
+testBundlers(
+  'Can require Node modules with destructuring expressions',
+  [ESBUILD, ESBUILD_ZISI, DEFAULT],
+  async (bundler, t) => {
+    await zipNode(t, `local-node-module-destructure-require`, {
+      opts: { config: { '*': { nodeBundler: bundler } } },
+    })
+
+    // TO DO: Remove when `parseWithEsbuild` feature flag is decommissioned.
+    await zipNode(t, `local-node-module-destructure-require`, {
+      opts: { config: { '*': { nodeBundler: bundler } }, featureFlags: { parseWithEsbuild: true } },
+    })
+  },
+)
+
 testBundlers('Can require scoped node modules', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
   await zipNode(t, 'node-module-scope', { opts: { config: { '*': { nodeBundler: bundler } } } })
 })