Repository: netlify/zip-it-and-ship-it
Pull request #582 (2021-07-15 14:01:51)

feat: use double asterisk in dynamic import shim glob
**- Summary**

When processing dynamic import expressions, we create glob expressions by replacing variables with a wildcard character. For example:

- `./lang/${variable}.json` -> `./lang/*.json`
- `./scripts/${variable}` -> `./scripts/*`

By using `*` as the wildcard character, it means we'll only include the first level of files. So in the second example above, a file under a nested directory (e.g. `scripts/nested/foo.js`) would be left out.

This PR changes the wildcard character to `**` so that nested directories are also included.

**- Test plan**

Added new tests

**- A picture of a cute animal (not mandatory but encouraged)**

🦁 


Patch:
diff --git a/src/runtimes/node/dynamic_imports/parser.js b/src/runtimes/node/dynamic_imports/parser.js
index d710bcb..b0248fd 100644
--- a/src/runtimes/node/dynamic_imports/parser.js
+++ b/src/runtimes/node/dynamic_imports/parser.js
@@ -3,7 +3,7 @@ const { join, relative, resolve } = require('path')
 const acorn = require('acorn')
 
 const ECMA_VERSION = 2021
-const GLOB_WILDCARD = '*'
+const GLOB_WILDCARD = '**'
 
 // Transforms an array of glob nodes into a glob string including an absolute
 // path.
@@ -30,7 +30,7 @@ const getAbsoluteGlob = ({ basePath, globNodes, resolveDir }) => {
 // expression and convertable to a wildcard character. This determines whether
 // we convert an expression or leave it alone. For example:
 //
-// - `./files/${someVariable}`: Convert `someVariable` to `*`
+// - `./files/${someVariable}`: Convert `someVariable` to GLOB_WILDCARD
 // - `./files/${[some, array]}`: Don't convert expression
 //
 // The following AST nodes are converted to a wildcard:
@@ -100,7 +100,7 @@ const parseRequire = ({ basePath, expression, resolveDir }) => {
 // static parts will be left untouched and identifiers will be replaced by
 // `GLOB_WILDCARD`.
 //
-// Example: './files/' + lang + '.json' => ["./files/", "*", ".json"]
+// Example: './files/' + lang + '.json' => ["./files/", "**", ".json"]
 const parseBinaryExpression = (expression) => {
   const { left, operator, right } = expression
 
@@ -128,7 +128,7 @@ const parseBinaryExpression = (expression) => {
 // static parts will be left untouched and identifiers will be replaced by
 // `GLOB_WILDCARD`.
 //
-// Example: `./files/${lang}.json` => ["./files/", "*", ".json"]
+// Example: `./files/${lang}.json` => ["./files/", "**", ".json"]
 const parseTemplateLiteral = (expression) => {
   const { expressions, quasis } = expression
   const parts = [...expressions, ...quasis].sort((partA, partB) => partA.start - partB.start)
diff --git a/src/runtimes/node/src_files.js b/src/runtimes/node/src_files.js
index be259b3..bfcd759 100644
--- a/src/runtimes/node/src_files.js
+++ b/src/runtimes/node/src_files.js
@@ -30,7 +30,7 @@ const getPathsOfIncludedFiles = async (includedFiles, basePath) => {
     { include: [], exclude: [] },
   )
   const pathGroups = await Promise.all(
-    include.map((expression) => pGlob(expression, { absolute: true, cwd: basePath, ignore: exclude })),
+    include.map((expression) => pGlob(expression, { absolute: true, cwd: basePath, ignore: exclude, nodir: true })),
   )
 
   // `pathGroups` is an array containing the paths for each expression in the


Test patch:
diff --git a/tests/fixtures/node-module-dynamic-import-4/function.js b/tests/fixtures/node-module-dynamic-import-4/function.js
new file mode 100644
index 0000000..89cb367
--- /dev/null
+++ b/tests/fixtures/node-module-dynamic-import-4/function.js
@@ -0,0 +1,4 @@
+const id = (arg) => arg
+
+// eslint-disable-next-line import/no-dynamic-require, node/global-require, prefer-template
+module.exports = (lang) => [require('./lang/' + lang), require('./lang/' + id(lang))]
diff --git a/tests/fixtures/node-module-dynamic-import-4/lang/en.js b/tests/fixtures/node-module-dynamic-import-4/lang/en.js
new file mode 100644
index 0000000..e70c3fe
--- /dev/null
+++ b/tests/fixtures/node-module-dynamic-import-4/lang/en.js
@@ -0,0 +1,1 @@
+module.exports = ['yes', 'no']
diff --git a/tests/fixtures/node-module-dynamic-import-4/lang/nested/es.js b/tests/fixtures/node-module-dynamic-import-4/lang/nested/es.js
new file mode 100644
index 0000000..f50efcf
--- /dev/null
+++ b/tests/fixtures/node-module-dynamic-import-4/lang/nested/es.js
@@ -0,0 +1,1 @@
+module.exports = ['sí', 'no']
diff --git a/tests/fixtures/node-module-dynamic-import-4/lang/pt.js b/tests/fixtures/node-module-dynamic-import-4/lang/pt.js
new file mode 100644
index 0000000..c69afba
--- /dev/null
+++ b/tests/fixtures/node-module-dynamic-import-4/lang/pt.js
@@ -0,0 +1,1 @@
+module.exports = ['sim', 'não']
diff --git a/tests/fixtures/node-module-dynamic-import-4/package.json b/tests/fixtures/node-module-dynamic-import-4/package.json
new file mode 100644
index 0000000..d0d4274
--- /dev/null
+++ b/tests/fixtures/node-module-dynamic-import-4/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "node-module-dynamic-import-2",
+  "version": "1.0.0"
+}
diff --git a/tests/main.js b/tests/main.js
index d8d7453..72adc02 100644
--- a/tests/main.js
+++ b/tests/main.js
@@ -1446,6 +1446,49 @@ test('Adds a runtime shim and includes the files needed for dynamic imports usin
   t.throws(() => func('fr'))
 })
 
+test('The dynamic import runtime shim handles files in nested directories', async (t) => {
+  const fixtureName = 'node-module-dynamic-import-4'
+  const { tmpDir } = await zipNode(t, fixtureName, {
+    opts: {
+      basePath: join(FIXTURES_DIR, fixtureName),
+      config: { '*': { nodeBundler: ESBUILD, processDynamicNodeImports: true } },
+    },
+  })
+
+  // eslint-disable-next-line import/no-dynamic-require, node/global-require
+  const func = require(`${tmpDir}/function.js`)
+
+  t.deepEqual(func('en')[0], ['yes', 'no'])
+  t.deepEqual(func('en')[1], ['yes', 'no'])
+  t.deepEqual(func('pt')[0], ['sim', 'não'])
+  t.deepEqual(func('pt')[1], ['sim', 'não'])
+  t.deepEqual(func('nested/es')[0], ['sí', 'no'])
+  t.deepEqual(func('nested/es')[1], ['sí', 'no'])
+  t.throws(() => func('fr'))
+})
+
+test('The dynamic import runtime shim handles files in nested directories when using `archiveType: "none"`', async (t) => {
+  const fixtureName = 'node-module-dynamic-import-4'
+  const { tmpDir } = await zipNode(t, fixtureName, {
+    opts: {
+      archiveFormat: 'none',
+      basePath: join(FIXTURES_DIR, fixtureName),
+      config: { '*': { nodeBundler: ESBUILD, processDynamicNodeImports: true } },
+    },
+  })
+
+  // eslint-disable-next-line import/no-dynamic-require, node/global-require
+  const func = require(`${tmpDir}/function/function.js`)
+
+  t.deepEqual(func('en')[0], ['yes', 'no'])
+  t.deepEqual(func('en')[1], ['yes', 'no'])
+  t.deepEqual(func('pt')[0], ['sim', 'não'])
+  t.deepEqual(func('pt')[1], ['sim', 'não'])
+  t.deepEqual(func('nested/es')[0], ['sí', 'no'])
+  t.deepEqual(func('nested/es')[1], ['sí', 'no'])
+  t.throws(() => func('fr'))
+})
+
 test('Negated files in `included_files` are excluded from the bundle even if they match a dynamic import expression', async (t) => {
   const fixtureName = 'node-module-dynamic-import-2'
   const { tmpDir } = await zipNode(t, fixtureName, {