Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use import instead of readFile to get content of Vite config file #125

Closed
srguiwiz2 opened this issue Mar 14, 2024 · 3 comments
Closed

Use import instead of readFile to get content of Vite config file #125

srguiwiz2 opened this issue Mar 14, 2024 · 3 comments

Comments

@srguiwiz2
Copy link
Contributor

srguiwiz2 commented Mar 14, 2024

This issue suggests using a JavaScript dynamic import as documented at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import instead of fs.readFileSync(getViteConfigPath()) .

The purpose is to allow a vite.config.js to have actual code create the base and outDir field from a common source. The currently in use regular expression would not handle that. In such a vite.config.js there could be source code like:

const projectName = "projekt";
export default defineConfig({
  plugins: [
    vue(),
  ],
  base: `/${projectName}/`,
  build: {
    outDir: `dist-${projectName}`
  },
})

This issue therefore suggests to replace file reading and regular expression matching with code like:

const viteConfigImportPath = path.resolve(process.cwd(), getViteConfigPath());
const viteConfigFromFile = (await import(viteConfigImportPath)).default;
const base = viteConfigFromFile.base;

Obviously I haven't addressed vite.config.ts and one fallback could be for that one to still read it as file. Shouldn't prevent reading vite.config.js the new way.

I haven't coded this out because it isn't an urgency for me yet. Therefore it could be details have to be different. Also because it is your project I suggest it here first, in case such a change is something you prefer doing yourself. I don't expect to want this before weeks from now or months from now.

@srguiwiz2
Copy link
Contributor Author

I have coded and am using a patch in JavaScript, and here it written in hopefully correct TypeScript:

    let root, base, outDir;
    try {
      const importConfigPath = path.resolve(process.cwd(), getViteConfigPath());
      const importedConfig: ViteConfig = (await import(importConfigPath)).default;
      root = importedConfig.root;
      base = importedConfig.base;
      outDir = importedConfig.build.outDir;
    } catch {
      const config = fs.readFileSync(getViteConfigPath(), "utf8");
      root = config.match(/"?root"?\s*:\s*"([^"]+)"/)?.[1];
      base = config.match(/"?base"?\s*:\s*"([^"]+)"/)?.[1];
      outDir = config.match(/"?outDir"?\s*:\s*"([^"]+)"/)?.[1];
    }

The diff I would commit would be:

diff --git a/src/main.ts b/src/main.ts
index 3f77f6f..20f2696 100644
--- a/src/main.ts
+++ b/src/main.ts
@@ -5,6 +5,7 @@ import http from "http";
 import https from "https";
 import path from "path";
 import pc from "picocolors";
+import process from "process";
 import type { ViteDevServer } from "vite";
 
 type ViteConfig = {
@@ -109,11 +110,19 @@ async function resolveConfig(): Promise<ViteConfig> {
   }
 
   try {
-    const config = fs.readFileSync(getViteConfigPath(), "utf8");
-
-    const root = config.match(/"?root"?\s*:\s*"([^"]+)"/)?.[1];
-    const base = config.match(/"?base"?\s*:\s*"([^"]+)"/)?.[1];
-    const outDir = config.match(/"?outDir"?\s*:\s*"([^"]+)"/)?.[1];
+    let root, base, outDir;
+    try {
+      const importConfigPath = path.resolve(process.cwd(), getViteConfigPath());
+      const importedConfig: ViteConfig = (await import(importConfigPath)).default;
+      root = importedConfig.root;
+      base = importedConfig.base;
+      outDir = importedConfig.build.outDir;
+    } catch {
+      const config = fs.readFileSync(getViteConfigPath(), "utf8");
+      root = config.match(/"?root"?\s*:\s*"([^"]+)"/)?.[1];
+      base = config.match(/"?base"?\s*:\s*"([^"]+)"/)?.[1];
+      outDir = config.match(/"?outDir"?\s*:\s*"([^"]+)"/)?.[1];
+    }
 
     const defaultConfig = getDefaultViteConfig();

This implementation is conservative in that it only kicks in if viteless, and in case it fails the preexisting code runs instead. Afaik this implementation only provides its better functionality if there is a vite.config.js. But then it is nice to help eliminate the need for having and updating the same information more than one place.

I could make a pull request for this one too. It could be easier on branch master after you approve or reject pull request #124, which on my master is in line in front of this change. Just let me know what's your preference.

@srguiwiz2
Copy link
Contributor Author

I have come to recognize this might not be working because it might or would fail in vite.config.js on the line

import { defineConfig } from 'vite'

Oh well. Some time wasted. But it was educational. It worked in my temporary dev environment. Feel free to check on your own, but I think it won't work.

Back to just using the code from pull request #124 . Looking forward to it getting approved.

@srguiwiz2
Copy link
Contributor Author

Withdrawing suggestion for reasons mentioned.

@srguiwiz2 srguiwiz2 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant