chore: forbid rc deep imports in build#18
Conversation
总体概览该 PR 更新了 ESLint 调用以动态构建 no-restricted-imports 规则集，限制来自 变更内容ESLint 导入限制规则
可能相关的 PR
诗歌
🎯 2 (Simple) | ⏱️ ~10 分钟 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/index.ts`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| 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}"`, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src && head -n 80 index.ts | tail -n 20Repository: react-component/father-plugin
Length of output: 679
🏁 Script executed:
cat -n src/index.ts | head -n 100Repository: 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.
| 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.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a new check to prevent importing package internals from es or lib directories for @rc-component and rc-* packages using the ESLint no-restricted-imports rule. Feedback indicates that the ESLint CLI's --rule flag does not support a single JSON object containing multiple rules. It is recommended to define the rules in a plain object and iterate over them to provide individual --rule flags in the command execution, which also helps avoid potential issues with manual string escaping across different shell environments.
| 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, '\\"'); |
There was a problem hiding this comment.
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.
| 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.', | |
| }, | |
| ], | |
| }, | |
| ], | |
| }; |
| 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}"`, |
There was a problem hiding this comment.
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}`,
|
Refs ant-design/ant-design#58115 antd 侧统一跟踪 rc 包 es/lib 深路径引用问题。 |
2 participants
背景
rc 包之间不应该依赖其他包的
es/lib构建产物内部路径。此前这类限制需要在各个包的 ESLint 配置里重复添加，容易遗漏。改动内容
@rc-component/father-plugin的 build 阶段 ESLint 检查中加入no-restricted-imports规则。@rc-component/*和rc-*包的es/lib深路径导入。@typescript-eslint/consistent-type-exports检查，并仍然只扫描.ts/.tsx。验证
npm run lint:esnpm run buildrc-motion中安装本地 tarball 后执行npm run compile，正常通过。rc-motion中临时添加@rc-component/util/es/raf导入后，npm run compile会被新增规则拦截；删除临时文件后恢复通过。Summary by CodeRabbit
发布说明