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

feat(core): add barrel-less modules and checks #149

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 45 additions & 25 deletions packages/core/src/lib/checks/check-for-deep-imports.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { FsPath } from '../file-info/fs-path';
import { SheriffConfig } from '../config/sheriff-config';
import { ProjectInfo } from '../main/init';
import { FileInfo } from "../modules/file.info";
import { FileInfo } from '../modules/file.info';

/**
* verifies if an existing file has deep imports which are forbidden.
Expand All @@ -16,17 +16,15 @@ export function checkForDeepImports(
const deepImports: string[] = [];
const assignedFileInfo = getFileInfo(fsPath);

const isRootAndExcluded = createIsRootAndExcluded(rootDir, config);
const isModuleBarrel = (fileInfo: FileInfo) =>
fileInfo.moduleInfo.hasBarrel &&
fileInfo.moduleInfo.barrelPath === fileInfo.path;

for (const importedFileInfo of assignedFileInfo.imports) {
if (
!isModuleBarrel(importedFileInfo) &&
!isRootAndExcluded(importedFileInfo.moduleInfo.path) &&
importedFileInfo.moduleInfo !== assignedFileInfo.moduleInfo
isSameModule(importedFileInfo, assignedFileInfo) ||
isExcludedRootModule(rootDir, config, importedFileInfo) ||
accessesBarrelFileForBarrelModules(importedFileInfo) ||
accessesExposedFileForBarrelLessModules(importedFileInfo, config.enableBarrelLess)
) {
// 👍 all good
} else {
deepImports.push(
assignedFileInfo.getRawImportForImportedFileInfo(importedFileInfo.path),
);
Expand All @@ -36,22 +34,44 @@ export function checkForDeepImports(
return deepImports;
}

/**
* creates a function which allows a deep import, if
* `excludeRoot` in the config is `true` and the
* importedModulePath is the root module.
*/
const createIsRootAndExcluded = (
function accessesExposedFileForBarrelLessModules(fileInfo: FileInfo, enableBarrelLess: boolean) {
if (!enableBarrelLess) {
return false;
}

if (fileInfo.moduleInfo.hasBarrel) {
return false;
}

const possibleEncapsulatedFolderPath =
fileInfo.moduleInfo.getEncapsulatedFolder();
if (possibleEncapsulatedFolderPath === undefined) {
return true;
}

return !fileInfo.path.startsWith(possibleEncapsulatedFolderPath);
}

function accessesBarrelFileForBarrelModules(fileInfo: FileInfo) {
if (!fileInfo.moduleInfo.hasBarrel) {
return false;
}

return fileInfo.moduleInfo.barrelPath === fileInfo.path;
}

function isExcludedRootModule(
rootDir: FsPath,
config: SheriffConfig | undefined,
) => {
let excludeRoot = false;
if (config === undefined) {
excludeRoot = false;
} else {
excludeRoot = Boolean(config.excludeRoot);
config: SheriffConfig,
importedModule: FileInfo,
) {
if (importedModule.moduleInfo.path !== rootDir) {
return false;
}

return (importedModulePath: string): boolean =>
excludeRoot && importedModulePath === rootDir;
};
return config.excludeRoot;
}

function isSameModule(importedFileInfo: FileInfo, assignedFileInfo: FileInfo) {
return importedFileInfo.moduleInfo.path === assignedFileInfo.moduleInfo.path
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
import { describe, it, expect } from 'vitest';
import { testInit } from '../../test/test-init';
import { tsConfig } from '../../test/fixtures/ts-config';
import { FileTree, sheriffConfig } from '../../test/project-configurator';
import { checkForDeepImports } from '../check-for-deep-imports';
import { UserSheriffConfig } from '../../config/user-sheriff-config';
import { traverseFileInfo } from '../../modules/traverse-file-info';

describe('barrel-less', () => {
it('should check for deep imports', () => {
assertProject()
.withCustomerRoute({
feature: {
internal: {
'customer-sub.component.ts': [],
},
'customer.component.ts': [
'./internal/customer-sub.component.ts',
'../data/customer.service.ts',
],
'customers.component.ts': ['../data/internal/hidden.service.ts'],
},
data: {
'customer.service.ts': ['./internal/hidden.service.ts'],
internal: { 'hidden.service.ts': [] },
},
})
.hasDeepImports({
'feature/customers.component.ts': [
'../data/internal/hidden.service.ts',
],
});
});

it('should also work with nested paths inside internal', () => {
assertProject()
.withCustomerRoute({
feature: {
internal: {
'customer-sub.component.ts': [],
},
'customer.component.ts': [
'./internal/customer-sub.component.ts',
'../data/customer.service.ts',
],
'customers.component.ts': [
'../data/internal/services/hidden.service.ts',
],
},
data: {
'customer.service.ts': ['./internal/services/hidden.service.ts'],
internal: { services: { 'hidden.service.ts': [] } },
},
})
.hasDeepImports({
'feature/customers.component.ts': [
'../data/internal/services/hidden.service.ts',
],
});
});

it('should also work with nested paths outside of internal', () => {
assertProject()
.withCustomerRoute({
feature: {
internal: {
'customer-sub.component.ts': [],
},
'customers.component.ts': ['./components/customers-sub.component.ts'],
'customer.component.ts': ['../data/services/customer.service.ts'],
components: {
'customers-sub.component.ts': [
'../internal/customer-sub.component.ts',
'../../data/internal/hidden.service.ts',
],
},
},
data: {
services: {
'customer.service.ts': ['../internal/hidden.service.ts'],
},
internal: { 'hidden.service.ts': [] },
},
})
.hasDeepImports({
'feature/components/customers-sub.component.ts': [
'../../data/internal/hidden.service.ts',
],
});
});

it('should be able to change the name of internals', () => {
assertProject({
encapsulatedFolderNameForBarrelLess: 'private',
})
.withCustomerRoute({
feature: {
'customer.component.ts': ['../data/internal/customer.service.ts'],
'customers.component.ts': ['../data/private/hidden.service.ts'],
},
data: {
private: { 'hidden.service.ts': [] },
internal: { 'customer.service.ts': ['../private/hidden.service.ts'] },
},
})
.hasDeepImports({
'feature/customers.component.ts': ['../data/private/hidden.service.ts'],
});
});

it('should always prioritize the barrel file', () => {
assertProject({ showWarningOnBarrelCollision: false })
.withCustomerRoute({
feature: {
'customer.component.ts': ['../data'],
'customers.component.ts': ['../data/open.service.ts'],
},
data: {
'index.ts': ['open.service.ts'],
'open.service.ts': [],
internal: { 'hidden.service.ts': [] },
},
})
.hasDeepImports({
'feature/customers.component.ts': ['../data/open.service.ts'],
});
});
});

function assertProject(config: Partial<UserSheriffConfig> = {}) {
return {
withCustomerRoute(customerFileTree: FileTree) {
return {
hasDeepImports(deepImports: Record<string, string[]> = {}) {
const projectInfo = testInit('src/main.ts', {
'tsconfig.json': tsConfig(),
'sheriff.config.ts': sheriffConfig({
...{
tagging: {
'src/app/<domain>/<type>': ['domain:<domain>', 'type:<type>'],
},
depRules: {},
enableBarrelLess: true,
},
...config,
}),
src: {
'main.ts': ['./app/app.routes'],
app: {
'app.routes.ts': [
'./customer/feature/customer.component.ts',
'./customer/feature/customers.component.ts',
],
customer: customerFileTree,
},
},
});

for (const { fileInfo } of traverseFileInfo(projectInfo.fileInfo)) {
expect(
fileInfo.hasUnresolvedImports(),
`${fileInfo.path} has unresolved imports`,
).toBe(false);

const pathToLookup = fileInfo.path.replace(
'/project/src/app/customer/',
'',
);

const expectedDeepImports = deepImports[pathToLookup] || [];
expect
.soft(
checkForDeepImports(fileInfo.path, projectInfo),
`deep imports check failed for ${fileInfo.path}`,
)
.toEqual(expectedDeepImports);
}
},
};
},
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('check deep imports', () => {
]) {
expect(
checkForDeepImports(toFsPath(`/project/${filename}`), projectInfo),
`failed deep import test for ${filename}`
`failed deep import test for ${filename}`,
).toEqual(deepImports);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,58 @@ describe('check for dependency rule violation', () => {
toFsPath('/project/src/customers/feature/customer.component.ts'),
projectInfo,
);
expect(violationsForSubFolder).toHaveLength(1)
expect(violationsForSubFolder).toHaveLength(1);
});

describe('barrel less', () => {
const setup = (addRuleForRoot: boolean) => {
return testInit('src/main.ts', {
'tsconfig.json': tsConfig(),
'sheriff.config.ts': sheriffConfig({
tagging: {
'src/customers': ['domain:customers', 'type:domain'],
'src/customers/<type>': ['domain:customers', 'type:<type>'],
},
depRules: {
'domain:customers': sameTag,
'type:domain': ['type:feature'],
'type:feature': [],
root: addRuleForRoot ? ['domain:*', 'type:feature'] : [],
},
enableBarrelLess: true,
}),
src: {
'main.ts': ['./customers/feature/customer.component.ts'],
customers: {
feature: {
'customer.routes.ts': ['./customer.component.ts'],
'customer.component.ts': ['../data/customer.service.ts'],
},
data: {
'customer.service.ts': [],
},
},
},
});
};

it('should test a default case', () => {
const projectInfo = setup(true);
const violationsForSuperFolder = checkForDependencyRuleViolation(
toFsPath('/project/src/main.ts'),
projectInfo,
);
expect(violationsForSuperFolder).toEqual([]);
});

it('should fail for dependency violation', () => {
const projectInfo = setup(false);
const violations = checkForDependencyRuleViolation(
toFsPath('/project/src/main.ts'),
projectInfo,
);
expect(violations).toHaveLength(1);
expect(violations[0]).toMatchObject({fromTag: 'root', toTags: ['domain:customers', 'type:feature']});
});
});
});
Loading
Loading