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.

Sign up for GitHub

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

支持命令行中的配置覆盖配置文件中的配置 #5350

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

支持命令行中的配置覆盖配置文件中的配置 #5350

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion internal/vite-config/src/utils/env.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,33 @@ function getConfFiles() {
return ['.env', '.env.local', `.env.${mode}`, `.env.${mode}.local`];
}

const regex = /(\w+)=(\w+)/g;
/**
* 获取命令行中传入的参数
*/
function getConfByScript() {
const script = process.env.npm_lifecycle_script;
const scriptEnv: Record<string, string> = {};
let matcher;
while (true) {
matcher = regex.exec(script as string);
if (matcher === null) {
break;
}
if (matcher.index === regex.lastIndex) {
regex.lastIndex++;
}

if (matcher) {
const key = matcher[1] as string;
scriptEnv[key] = matcher[2] as string;
}
}

return scriptEnv;
}
Comment on lines +32 to +56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve robustness of command-line parameter parsing.

The current implementation has several limitations and potential issues:

  1. The regex pattern is too restrictive and doesn't support common npm script parameter formats
  2. The while(true) loop with regex.exec() could be problematic
  3. Contains redundant checks and lacks proper error handling

Consider this safer implementation:

-const regex = /(\w+)=(\w+)/g;
+const regex = /([A-Za-z][\w-]*)=(["']?)([^\n"']*)\2/g;

 function getConfByScript() {
   const script = process.env.npm_lifecycle_script;
+  if (!script) {
+    return {};
+  }
+
   const scriptEnv: Record<string, string> = {};
-  let matcher;
-  while (true) {
-    matcher = regex.exec(script as string);
-    if (matcher === null) {
-      break;
-    }
-    if (matcher.index === regex.lastIndex) {
-      regex.lastIndex++;
-    }
-
-    if (matcher) {
-      const key = matcher[1] as string;
-      scriptEnv[key] = matcher[2] as string;
-    }
+  for (const match of script.matchAll(regex)) {
+    const [, key, , value] = match;
+    scriptEnv[key] = value;
   }

   return scriptEnv;
 }

The improved version:

  • Supports quoted values and hyphenated keys
  • Uses for...of with matchAll() instead of while(true)
  • Adds null check for npm_lifecycle_script
  • Properly destructures regex groups
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const regex = /(\w+)=(\w+)/g;
/**
* 获取命令行中传入的参数
*/
function getConfByScript() {
const script = process.env.npm_lifecycle_script;
const scriptEnv: Record<string, string> = {};
let matcher;
while (true) {
matcher = regex.exec(script as string);
if (matcher === null) {
break;
}
if (matcher.index === regex.lastIndex) {
regex.lastIndex++;
}
if (matcher) {
const key = matcher[1] as string;
scriptEnv[key] = matcher[2] as string;
}
}
return scriptEnv;
}
const regex = /([A-Za-z][\w-]*)=(["']?)([^\n"']*)\2/g;
/**
* 获取命令行中传入的参数
*/
function getConfByScript() {
const script = process.env.npm_lifecycle_script;
if (!script) {
return {};
}
const scriptEnv: Record<string, string> = {};
for (const match of script.matchAll(regex)) {
const [, key, , value] = match;
scriptEnv[key] = value;
}
return scriptEnv;
}



/**
* Get the environment variables starting with the specified prefix
* @param match prefix
Expand All @@ -39,7 +66,8 @@ async function loadEnv<T = Record<string, string>>(
confFiles = getConfFiles(),
) {
let envConfig = {};


// 配置文件中的配置
for (const confFile of confFiles) {
try {
const confFilePath = join(process.cwd(), confFile);
Expand All @@ -54,6 +82,12 @@ async function loadEnv<T = Record<string, string>>(
console.error(`Error while parsing ${confFile}`, error);
}
}

// 命令行中的配置
const scriptConfig = getConfByScript();
envConfig = { ...envConfig, ...scriptConfig };
console.log('命令行中的参数已经覆盖配置文件中的参数 %o', scriptConfig);

const reg = new RegExp(`^(${match})`);
Object.keys(envConfig).forEach((key) => {
if (!reg.test(key)) {
Expand Down
Loading