fastify / fastify-autoload

Require all plugins in a directory
MIT License
333 stars 67 forks source link

Investigate if readFileSync is faster #385

Open Uzlopak opened 5 months ago

Uzlopak commented 5 months ago

Prerequisites

Issue

I wonder if we could speed up the whole loading process, if we load the files sync and not async.

From e0c3c8009ef85fc5b13e2c48323b67bf5536339c Mon Sep 17 00:00:00 2001
From: uzlopak <aras.abbasi@googlemail.com>
Date: Mon, 17 Jun 2024 12:14:50 +0200
Subject: [PATCH] readFileSync

---
 index.js | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/index.js b/index.js
index 0714843..2dd961d 100644
--- a/index.js
+++ b/index.js
@@ -1,7 +1,7 @@
 'use strict'

-const { promises: { readdir, readFile } } = require('node:fs')
-const { join, relative, sep } = require('node:path')
+const { promises: { readdir }, readFileSync } = require('node:fs')
+const { join: pathJoin, relative, sep } = require('node:path')
 const { pathToFileURL } = require('node:url')
 const runtime = require('./runtime')

@@ -17,7 +17,7 @@ const defaults = {
 }

 const fastifyAutoload = async function autoload (fastify, options) {
-  const packageType = await getPackageType(options.dir)
+  const packageType = getPackageType(options.dir)
   const opts = { ...defaults, packageType, ...options }
   const pluginTree = await findPlugins(opts.dir, opts)
   const pluginsMeta = {}
@@ -97,23 +97,37 @@ const fastifyAutoload = async function autoload (fastify, options) {
   }
 }

-async function getPackageType (cwd) {
+/**
+ * Get the package type from the nearest package.json file
+ *
+ * @param {string} cwd The current working directory
+ * @returns {Promise<string>}
+ */
+function getPackageType (cwd) {
   const directories = cwd.split(sep)

   // required for paths that begin with the sep, such as linux root
-  directories[0] = /* istanbul ignore next - OS specific */ directories[0] !== '' ? directories[0] : sep
+  /* istanbul ignore next - OS specific */
+  if (directories[0] === '') {
+    directories[0] = sep
+  }
+
+  let fileContents = null

   while (directories.length > 0) {
-    const filePath = join(...directories, 'package.json')
+    const filePath = pathJoin(...directories, 'package.json')

-    const fileContents = await readFile(filePath, 'utf-8')
-      .catch(() => null)
+    try {
+      fileContents = readFileSync(filePath, 'utf-8')
+    } catch (err) {
+      fileContents = null
+    }

     if (fileContents) {
       return JSON.parse(fileContents).type
     }

-    directories.pop()
+    directories.length = directories.length - 1
   }
 }

@@ -147,7 +161,7 @@ async function findPlugins (dir, options, hookedAccumulator = {}, prefix, depth
     // Contains autohooks file?
     const autoHooks = list.find((dirent) => autoHooksPattern.test(dirent.name))
     if (autoHooks) {
-      const autoHooksFile = join(dir, autoHooks.name)
+      const autoHooksFile = pathJoin(dir, autoHooks.name)
       const { type: autoHooksType } = getScriptType(autoHooksFile, options.packageType)

       // Overwrite current hooks?
@@ -165,7 +179,7 @@ async function findPlugins (dir, options, hookedAccumulator = {}, prefix, depth
   // Contains index file?
   const indexDirent = list.find((dirent) => indexPattern.test(dirent.name))
   if (indexDirent) {
-    const file = join(dir, indexDirent.name)
+    const file = pathJoin(dir, indexDirent.name)
     const { language, type } = getScriptType(file, options.packageType)
     if (language === 'typescript' && !runtime.supportTypeScript) {
       throw new Error(`@fastify/autoload cannot import hooks plugin at '${file}'. To fix this error compile TypeScript to JavaScript or use 'ts-node' to run your app.`)
@@ -193,7 +207,7 @@ async function findPlugins (dir, options, hookedAccumulator = {}, prefix, depth
     }

     const atMaxDepth = Number.isFinite(maxDepth) && maxDepth <= depth
-    const file = join(dir, dirent.name)
+    const file = pathJoin(dir, dirent.name)
     if (dirent.isDirectory() && !atMaxDepth) {
       let prefixBreadCrumb = (prefix ? `${prefix}/` : '/')
       if (dirNameRoutePrefix === true) {
-- 
2.34.1
climba03003 commented 5 months ago

I believe the answer is yes, but please aware it also block the whole event-loop waiting for file I/O. So, other plugins in application may stale for waiting this plugin to finish the job.

jean-michelet commented 4 months ago

What is the status of this issue? Will it lead to a refactoring?

If so, the conversation, imo, can be transferred to #383