Skip to content

Commit

Permalink
feat: add CASE_SENSITIVE_PATHS doctor rule (#621)
Browse files Browse the repository at this point in the history
* feat: add CASE_SENSITIVE_PATHS doctor rule

* chore: ci test

* test: correct case for doctor

* refactor: improve doctor rule text

* test: update doctor case

* refactor: also check dependency import
  • Loading branch information
PeachScript committed Apr 4, 2023
1 parent 6629edd commit d4627a0
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 4 deletions.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@
"@umijs/babel-preset-umi": "^4.0.59",
"@umijs/bundler-utils": "^4.0.59",
"@umijs/bundler-webpack": "^4.0.59",
"@umijs/case-sensitive-paths-webpack-plugin": "^1.0.1",
"@umijs/core": "^4.0.59",
"@umijs/utils": "^4.0.59",
"@vercel/ncc": "0.33.3",
"babel-plugin-dynamic-import-node": "2.3.3",
"babel-plugin-module-resolver": "4.1.0",
"babel-plugin-react-require": "3.1.3",
"babel-plugin-transform-define": "2.0.1",
"enhanced-resolve": "5.9.3",
"fast-glob": "3.2.12",
"file-system-cache": "2.0.0",
"loader-runner": "4.2.0",
Expand Down
4 changes: 4 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

67 changes: 67 additions & 0 deletions src/doctor/rules/CASE_SENSITIVE_PATHS.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import type { IDoctorReport } from '..';
import type { IApi } from '../../types';
import fs from 'fs';
import enhancedResolve from 'enhanced-resolve';
import CaseSensitivePathsPlugin from '@umijs/case-sensitive-paths-webpack-plugin';
import { chalk } from '@umijs/utils';

export default (api: IApi) => {
const checker = new CaseSensitivePathsPlugin();
let resolver: ReturnType<typeof enhancedResolve['create']['sync']>;
let aliasKeys: string[];

// set fs for checker
checker.fs = fs;

api.addImportsCheckup(async ({ file, imports, mergedAlias }) => {
const errors: IDoctorReport = [];

resolver ??= enhancedResolve.create.sync({
// handle source extensions
extensions: ['.ts', '.js', '.tsx', '.jsx', '.json', '.node'],
mainFields: ['module', 'main', 'browser'],
// keep path clear in cnpm/pnpm project
symlinks: false,
alias: mergedAlias,
});
aliasKeys ??= Object.keys(mergedAlias);

for (const i of imports) {
let res: ReturnType<typeof resolver> = false;

// try to resolve import to absolute file path
try {
res = resolver(i.resolveDir, i.path);
} catch {
/* skip if module not found */
}

// check case sensitive
if (res) {
// why ignore next?
// because coverage will run on linux, and linux is case-sensitive
/* istanbul ignore next -- @preserve */
try {
checker.context =
// for npm package, use package root path as context, to avoid check node_modules/*
res.match(/^.+node_modules[\/](?:@[^\/]+[\/])?[^\/]+/)?.[0] ||
// for local file, use cwd as context
api.cwd;
await checker.checkFileExistsWithCase(res);
} catch (e: any) {
errors.push({
type: 'error',
problem: `${e.message
.replace(/\[.+?\] /, '')
.replace(/`.+?`/, `\`${i.path}\``)}
${chalk.gray(`at ${file}`)}`,
solution:
'Make sure that import path and filesystem path are exactly the same',
});
}
}
}

return errors;
});
};
4 changes: 2 additions & 2 deletions src/doctor/rules/PHANTOM_DEPS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ export default (api: IApi) => {
if (resolvedPath.includes('node_modules')) {
errors.push({
type: 'error',
problem: `Source depend on \`${pkgName}\` but it is not in the \`dependencies\` or \`peerDependencies\`
problem: `Source depends on \`${pkgName}\`, but there is no declaration of it
${chalk.gray(`at ${file}`)}`,
solution:
'Add it to the `dependencies` or `peerDependencies` of the package.json file',
'Add it to one of `dependencies`, `peerDependencies` and `optionalDependencies` in the package.json file',
});
}
}
Expand Down
11 changes: 10 additions & 1 deletion tests/doctor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ test('doctor: error checkups', async () => {

// PHANTOM_DEPS
expect(console.log).toHaveBeenCalledWith(
expect.stringContaining('Source depend on'),
expect.stringContaining('Source depends on'),
);

// PHANTOM_DEPS no standard library
Expand All @@ -75,6 +75,15 @@ test('doctor: error checkups', async () => {
expect.stringContaining('will not be published'),
);

// CASE_SENSITIVE_PATHS
// why only win32 and drawin?
// because Windows and macOS are case-insensitive by default
if (['win32', 'drawin'].includes(process.platform)) {
expect(console.log).toHaveBeenCalledWith(
expect.stringContaining('the corresponding path'),
);
}

// process.exit(1)
expect(mockExit).toHaveBeenCalledWith(1);
});
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/doctor/errors/src/camelCase.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 1;
11 changes: 10 additions & 1 deletion tests/fixtures/doctor/errors/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,16 @@ import alias from 'alias';
import path from 'child_process';
import externals from 'externals';
import hello from 'hello';
import BigCamelCase from './CamelCase';
import './index.less';

// to avoid esbuild tree-shaking
console.log(hello, alias, orgAlias, externals, orgExternals, path);
console.log(
hello,
alias,
orgAlias,
externals,
orgExternals,
path,
BigCamelCase,
);

0 comments on commit d4627a0

Please sign in to comment.