Fixing files imported by module got bypass through isPluginOrModule check (#463)

* FIX: error when autoload non-module file in forceESM mode

* FIX: error when autoload non-module file in forceESM mode

* FIX: use mjs file name for routes instead so it could load as ESM + move importedAsModule out of isPluginOrModule function (but include a boolean check for when an mjs is empty)

* UPDATE: add mock log count + more descriptive test name

* Update index.js

Co-authored-by: KaKa <[email protected]>
Signed-off-by: Aras Abbasi <[email protected]>

---------

Signed-off-by: Aras Abbasi <[email protected]>
Co-authored-by: thaivlinh <[email protected]>
Co-authored-by: Aras Abbasi <[email protected]>
Co-authored-by: KaKa <[email protected]>

Author: 4 people

index.js

@@ -67,14 +67,15 @@ async function loadPlugins ({ pluginTree, options, opts, fastify }) {
 
 async function loadPlugin ({ file, type, directoryPrefix, options, log }) {
   const { options: overrideConfig, forceESM, encapsulate } = options
+  const importedAsModule = forceESM || type === 'module' || runtime.forceESM
   let content
-  if (forceESM || type === 'module' || runtime.forceESM) {
+  if (importedAsModule) {
     content = await import(pathToFileURL(file).href)
   } else {
     content = require(file)
   }
 
-  if (isPluginOrModule(content) === false) {
+  if (isPluginOrModule(importedAsModule ? content.default : content) === false) {
     // We must have something that resembles a Fastify plugin function, or that
     // can be converted into one, that can eventually be passed to `avvio`. If
     // it is anything else, skip automatic loading of this item.
@@ -256,8 +257,9 @@ const pluginOrModulePattern = /\[object (?:AsyncFunction|Function|Module)\]/u
  * eventually passing it into `avvio`. False otherwise.
  */
 function isPluginOrModule (input) {
-  let result = false
+  if (!input) return false
 
+  let result = false
   const inputType = Object.prototype.toString.call(input)
   if (pluginOrModulePattern.test(inputType) === true) {
     result = true

test/issues/462/routes/api.mjs

@@ -0,0 +1,5 @@
+export default async function (app) {
+  app.get('/api', async function (req, reply) {
+    reply.status(200).send({ path: req.url })
+  })
+}

test/issues/462/routes/empty.cjs

@@ -0,0 +1 @@
+// cjs empty file

test/issues/462/routes/empty.js

@@ -0,0 +1 @@
+// empty file

test/issues/462/routes/empty.mjs

@@ -0,0 +1 @@
+// mjs empty file

test/issues/462/routes/hello.mjs

@@ -0,0 +1,4 @@
+// file with something but not a plugin export
+export const hello = () => {
+  // do something
+}

test/issues/462/routes/object.mjs

@@ -0,0 +1,2 @@
+// export default an empty object
+export default {}

test/issues/462/test.js

@@ -0,0 +1,37 @@
+'use strict'
+
+const { after, before, describe, it, mock } = require('node:test')
+const path = require('node:path')
+const Fastify = require('fastify')
+const autoLoad = require('../../../')
+const assert = require('node:assert')
+
+describe('Issue 462: ignore invalid and/or empty .js and .mjs files when using esm', function () {
+  const app = Fastify()
+
+  before(async function () {
+    app.register(autoLoad, {
+      dir: path.join(__dirname, 'routes'),
+      forceESM: true,
+    })
+    mock.method(app.log, 'debug')
+    await app.ready()
+  })
+
+  after(async function () {
+    await app.close()
+  })
+
+  it('should respond correctly to /api', async function () {
+    const res = await app.inject({ url: '/api' })
+
+    assert.strictEqual(res.statusCode, 200)
+    assert.deepStrictEqual(res.json(), { path: '/api' })
+
+    // check debug call when importing empty/invalid esm files
+    // 5 debug log including: empty.cjs, empty.js, empty.mjs, hello.mjs, object.mjs
+    // api.mjs pass the check so it will not trigger the debug log
+    // increasing this number if there are more empty/invalid esm files you want to test
+    assert.strictEqual(app.log.debug.mock.callCount(), 5)
+  })
+})