Split forbidden imports Into Seperate Rules#186
Split forbidden imports Into Seperate Rules#186illright merged 10 commits intofeature-sliced:masterfrom
forbidden imports Into Seperate Rules#186Conversation
🦋 Changeset detectedLatest commit: 948c6ea The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull Request Overview
This PR splits the existing forbidden-imports rule into two granular rules—no-higher-level-imports and no-cross-imports—to provide more flexible control over module import restrictions. It also introduces an enableSpecificRules function for selectively enabling rules in the recommended configuration.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/toolkit/src/index.ts | Added export of enableSpecificRules to support selective rule enablement. |
| packages/toolkit/src/create-configs.ts | Introduced the enableSpecificRules function and its associated tests. |
| packages/steiger-plugin-fsd/src/no-higher-level-imports/* | New rule implementation, tests, and documentation for forbidding imports from higher layers. |
| packages/steiger-plugin-fsd/src/no-cross-imports/* | New rule implementation, tests, and documentation for forbidding cross-imports between slices. |
| packages/steiger-plugin-fsd/src/index.ts | Updated plugin configuration to use the new enableSpecificRules function; combined enabled and disabled rules. |
| packages/steiger-plugin-fsd/src/forbidden-imports/README.md | Added a section detailing the split into related granular rules. |
| .changeset/* | Added changeset files documenting the updates and refactoring of rules. |
Comments suppressed due to low confidence (1)
packages/steiger-plugin-fsd/src/index.ts:42
- [nitpick] The variable name 'disabeldRules' appears to be misspelled; consider renaming it to 'disabledRules' for clarity.
const disabeldRules = [noCrossImports, noHigherLevelImports]
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the forbidden imports rule into two granular rules (no-higher-level-imports and no-cross-imports) while adding a new function (enableSpecificRules) to allow selective rule activation.
- Split forbidden-imports into two separate rules for finer control.
- Added enableSpecificRules to support enabling only specific rules via the recommended config.
- Updated plugin index to use enableSpecificRules and adjust rule grouping.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/toolkit/src/index.ts | Updated exports to include enableSpecificRules. |
| packages/toolkit/src/create-configs.ts | Added enableSpecificRules function with tests verifying its behavior. |
| packages/steiger-plugin-fsd/src/no-higher-level-imports/** | New rule and tests to prevent higher layer imports. |
| packages/steiger-plugin-fsd/src/no-cross-imports/** | New rule and tests to prevent cross-imports between slices. |
| packages/steiger-plugin-fsd/src/index.ts | Adjusted plugin assembly to separate and conditionally enable rules. |
| Packages documentation and changeset files | Updated documentation and release notes for the split rules and new function. |
Comments suppressed due to low confidence (1)
packages/toolkit/src/create-configs.ts:27
- [nitpick] Consider extracting the mapping of rule names from 'rulesToEnable' outside of the filter callback to avoid re-computing the array on every iteration, which will improve clarity and performance.
plugin.ruleDefinitions.filter((rule) => rulesToEnable.map((ruleToEnable) => ruleToEnable.name).includes(rule.name))
illright
left a comment
illright
left a comment
There was a problem hiding this comment.
Thank you for the quick PR! I have some suggestions, mostly around the documentation
gwbaik9717
force-pushed
the
split-forbidden-imports
branch
from
3270cb0 to
7a7d93e
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR splits the existing forbidden-imports rule into two more granular rules—no-higher-level-imports and no-cross-imports—to allow more precise import restriction enforcement while providing backward compatibility via a new selective enabling function. It also updates documentation, tests, and recommended configurations accordingly.
- Added and exported the new enableSpecificRules function in the toolkit package.
- Introduced two new granular rules for higher-level and cross imports with supporting tests and updated documentation.
- Updated the plugin index and changelogs to reflect the split and selective rule enabling.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/toolkit/src/index.ts | Exported enableSpecificRules along with existing API adjustments. |
| packages/toolkit/src/create-configs.ts | Implemented enableSpecificRules with accompanying tests for selective rule enabling. |
| packages/steiger-plugin-fsd/src/no-higher-level-imports/index.ts | Introduced a granular rule to prevent imports from higher layers. |
| packages/steiger-plugin-fsd/src/no-higher-level-imports/index.spec.ts | Added tests to verify the no-higher-level-imports functionality. |
| packages/steiger-plugin-fsd/src/no-higher-level-imports/README.md | Provided documentation on the no-higher-level-imports rule. |
| packages/steiger-plugin-fsd/src/no-cross-imports/index.ts | Introduced a granular rule to prevent cross-imports between slices. |
| packages/steiger-plugin-fsd/src/no-cross-imports/index.spec.ts | Added tests to verify the no-cross-imports functionality. |
| packages/steiger-plugin-fsd/src/no-cross-imports/README.md | Provided documentation on the no-cross-imports rule. |
| packages/steiger-plugin-fsd/src/index.ts | Updated exports and configuration to disable the new granular rules by default. |
| packages/steiger-plugin-fsd/src/forbidden-imports/README.md | Updated documentation reflecting the new split rules. |
| .changeset/*.md | Added changelog entries documenting the new functionality and split rules. |
|
Thank you for the detailed review! I think Copilot is great, but CodeRabbit could be a strong option as well. Would it be possible to consider integrating this into GitHub Actions? |
illright
left a comment
illright
left a comment
There was a problem hiding this comment.
Sorry for the delay in my response, thanks for addressing the comments! Just one tiny thing left.
About CodeRabbit — we could integrate it, but I don't see too much value from it. Frankly, I only requested the review from Copilot because I saw a new button and wanted to try it out, but I wasn't particularly happy with how it did. And I'll still review everything by hand anyway :)
illright
left a comment
illright
left a comment
There was a problem hiding this comment.
Thanks a lot! Let's get this merged
Description
1. Separated
forbidden-importsinto two more granular rules:no-higher-level-imports: Prevents imports from higher layersno-cross-imports: Prevents cross-imports between slices on the same layer2. Disable new rules in recommend config for default
enableSpecificRulesfunction which allows selectively enable only certain rulesFixes
Fixes #182