Skip to content

chore: forbid rc deep imports in build #18

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

Merged
zombieJ merged 1 commit into from
May 18, 2026
Merged

chore: forbid rc deep imports in build #18

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
31 changes: 29 additions & 2 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,33 @@ import path from 'path';

const cwd = process.cwd();

const restrictedPackageDirectoryImports = [
'@rc-component/*/es',
'@rc-component/*/es/**',
'@rc-component/*/lib',
'@rc-component/*/lib/**',
'rc-*/es',
'rc-*/es/**',
'rc-*/lib',
'rc-*/lib/**',
];

const eslintRulesString = JSON.stringify({
'@typescript-eslint/consistent-type-exports': 'error',
'no-restricted-imports': [
'error',
{
patterns: [
{
group: restrictedPackageDirectoryImports,
message:
'Do not import package internals from es/lib. Import from the package root.',
},
],
},
],
}).replace(/"/g, '\\"');
Comment on lines +19 to +33

@gemini-code-assist gemini-code-assist Bot May 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The ESLint CLI --rule option expects a single rule definition in the format rule-id: config. It does not support passing a JSON object containing multiple rules. Passing the entire object as a single string will cause ESLint to fail or ignore the rules. It is better to keep the rules as a plain object and format them individually when constructing the command.

Suggested change
const eslintRulesString = JSON.stringify({
'@typescript-eslint/consistent-type-exports': 'error',
'no-restricted-imports': [
'error',
{
patterns: [
{
group: restrictedPackageDirectoryImports,
message:
'Do not import package internals from es/lib. Import from the package root.',
},
],
},
],
}).replace(/"/g, '\\"');
const eslintRules = {
'@typescript-eslint/consistent-type-exports': 'error',
'no-restricted-imports': [
'error',
{
patterns: [
{
group: restrictedPackageDirectoryImports,
message:
'Do not import package internals from es/lib. Import from the package root.',
},
],
},
],
};


// 检查 package.json 中是否有指定的 npm 包依赖
function checkNpmPackageDependency(packageJson: any, packageName: string) {
return !!(
Expand All @@ -20,7 +47,7 @@ export default (api: IApi) => {
return;
}

console.log('Check Typescript exports...');
console.log('Check Typescript exports and rc package directory imports...');

// Break if current project not install `@rc-component/np`
const packageJson = await fs.readJson(path.join(cwd, 'package.json'));
Expand All @@ -40,7 +67,7 @@ export default (api: IApi) => {
if (isEslintInstalled) {
execSync(
// Requires compatibility with Windows environment
`npx eslint ${inputFolder} --ext .tsx,.ts --rule "@typescript-eslint/consistent-type-exports: error"`,
`npx eslint ${inputFolder} --ext .tsx,.ts --rule "${eslintRulesString}"`,
Comment on lines 68 to +70

@coderabbitai coderabbitai Bot May 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cd src && head -n 80 index.ts | tail -n 20

Repository: react-component/father-plugin

Length of output: 679

🏁 Script executed:

cat -n src/index.ts | head -n 100

Repository: react-component/father-plugin

Length of output: 3578

避免通过 shell 字符串拼接传递 --rule，Windows 转义不稳定

第 33 行代码对 JSON 使用了 .replace(/"/g, '\\"') 手动转义引号，然后在第 70 行将转义后的字符串通过 shell 命令字符串传入。这种方式在 Windows（尤其是 cmd.exe）和 Unix shell 中的转义行为不一致，容易导致 ESLint 规则被错误解析，与第 69 行"Windows 兼容"的注释目标相悖。

建议改为 spawnSync/execFileSync + 参数数组的方式，直接传递原始 JSON，避免 shell 解析和手动转义，确保跨平台一致性。

建议修改示例 
-import { execSync } from 'child_process';
+import { spawnSync } from 'child_process';

const cwd = process.cwd();

const restrictedPackageDirectoryImports = [
  '`@rc-component/`*/es',
  '`@rc-component/`*/es/**',
  '`@rc-component/`*/lib',
  '`@rc-component/`*/lib/**',
  'rc-*/es',
  'rc-*/es/**',
  'rc-*/lib',
  'rc-*/lib/**',
];

const eslintRulesString = JSON.stringify({
  '`@typescript-eslint/consistent-type-exports`': 'error',
  'no-restricted-imports': [
    'error',
    {
      patterns: [
        {
          group: restrictedPackageDirectoryImports,
          message:
            'Do not import package internals from es/lib. Import from the package root.',
        },
      ],
    },
  ],
-}).replace(/"/g, '\\"');

...

    const isEslintInstalled = checkNpmPackageDependency(packageJson, 'eslint');
    if (isEslintInstalled) {
-      execSync(
-        // Requires compatibility with Windows environment
-        `npx eslint ${inputFolder} --ext .tsx,.ts --rule "${eslintRulesString}"`,
+      const eslintBin = process.platform === 'win32' ? 'npx.cmd' : 'npx';
+      const result = spawnSync(
+        eslintBin,
+        [
+          'eslint',
+          inputFolder,
+          '--ext',
+          '.tsx,.ts',
+          '--rule',
+          eslintRulesString,
+        ],
         {
           cwd,
           env: process.env,
-          stdio: [process.stdin, process.stdout, process.stderr],
-          encoding: 'utf-8',
+          stdio: 'inherit',
         },
       );
+      if (result.status !== 0) {
+        process.exit(result.status ?? 1);
+      }
    } else {
      console.log('ESLint is not installed, skip.');
    }
📝 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
execSync(
// Requires compatibility with Windows environment
`npx eslint ${inputFolder} --ext .tsx,.ts --rule "@typescript-eslint/consistent-type-exports: error"`,
`npx eslint ${inputFolder} --ext .tsx,.ts --rule "${eslintRulesString}"`,
const eslintBin = process.platform === 'win32' ? 'npx.cmd' : 'npx';
const result = spawnSync(
eslintBin,
[
'eslint',
inputFolder,
'--ext',
'.tsx,.ts',
'--rule',
eslintRulesString,
],
{
cwd,
env: process.env,
stdio: 'inherit',
},
);
if (result.status !== 0) {
process.exit(result.status ?? 1);
}
🧰 Tools
🪛 OpenGrep (1.20.0)

[ERROR] 68-77: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.

(coderabbit.command-injection.exec-js)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/index.ts` around lines 68 - 70, The current use of execSync with a
shell-joined command (`execSync` invoking `npx eslint ${inputFolder} --ext
.tsx,.ts --rule "${eslintRulesString}"`) relies on manual JSON quoting and shell
escaping (`eslintRulesString`), which breaks on Windows; change to a spawn-style
call that passes arguments as an array to avoid shell parsing: replace the
`execSync` invocation with `spawnSync`/`execFileSync` (Node child_process) and
pass `['eslint', inputFolder, '--ext', '.tsx,.ts', '--rule', eslintRulesString]`
(or equivalent) so the raw JSON in `eslintRulesString` is forwarded directly;
update error handling to mirror the current sync behavior and keep the "Windows
compatibility" intent intact by removing manual `.replace(/"/g, '\\"')`
escaping.

Comment on lines 68 to +70

@gemini-code-assist gemini-code-assist Bot May 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Since the --rule flag must be specified for each rule individually, you should map over the eslintRules object to generate multiple --rule arguments. This also ensures that each rule is correctly formatted for the CLI. Note that manual string escaping for shell commands can be fragile across different environments (like Windows CMD vs. POSIX shells); using spawnSync with an array of arguments would be a more robust alternative in the future.

      const rulesArgs = Object.entries(eslintRules)
        .map(([rule, config]) => `--rule "${rule}: ${JSON.stringify(config).replace(/"/g, '\\"')}"`)
        .join(' ');
      execSync(
        // Requires compatibility with Windows environment
        `npx eslint ${inputFolder} --ext .tsx,.ts ${rulesArgs}`,

{
cwd,
env: process.env,
Expand Down