-
Notifications
You must be signed in to change notification settings - Fork 464
feat(nx-plugin): enforce module boundaries on new arch #16803
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
ed40779
feat(nx-plugin): infer boundary tags for new-arch packages
ysitbon cff540c
feat(nx-plugin): enforce module boundaries via project graph
ysitbon 43683f1
chore: address PR review — de-dup tag inference, dedicated boundary c…
ysitbon a61c52f
fix(nx-plugin): dedupe boundary violations per source/target edge
ysitbon 2e4ad31
chore: restore workflow pin version to develop
ysitbon 7c4b507
chore: delete unecessary changeset
ysitbon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| name: "[Module Boundaries] - Test - Called" | ||
|
|
||
| on: | ||
| workflow_call: | ||
| workflow_dispatch: | ||
| inputs: | ||
| ref: | ||
| description: | | ||
| If you run this manually, and want to run on a PR, the correct ref should be refs/pull/{PR_NUMBER}/merge to | ||
| have the "normal" scenario involving checking out a merge commit between your branch and the base branch. | ||
| If you want to run only on a branch or specific commit, you can use either the sha or the branch name instead (prefer the first version for PRs). | ||
| required: false | ||
|
|
||
| permissions: | ||
| id-token: write | ||
| contents: read | ||
|
|
||
| jobs: | ||
| enforce-boundaries: | ||
| name: Enforce Module Boundaries | ||
| runs-on: ubuntu-22.04 | ||
| timeout-minutes: 10 | ||
| env: | ||
| NODE_OPTIONS: "--max-old-space-size=4096" | ||
| CI_OS: ubuntu-22.04 | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 | ||
| with: | ||
| ref: ${{ inputs.ref || github.sha }} | ||
|
|
||
| - name: Setup the caches | ||
| uses: LedgerHQ/ledger-live/tools/actions/composites/setup-caches@develop | ||
| id: setup-caches | ||
| with: | ||
| use-mise: true | ||
| gh-token: ${{ secrets.GITHUB_TOKEN }} | ||
| cache-develop-role-arn: ${{ secrets.AWS_CACHE_OIDC_ROLE_ARN_DEVELOP }} | ||
| cache-branch-role-arn: ${{ secrets.AWS_CACHE_OIDC_ROLE_ARN_BRANCH }} | ||
| nx-key: ${{ secrets.NX_KEY }} | ||
| accountId: ${{ secrets.AWS_ACCOUNT_ID_PROD }} | ||
| roleName: ${{ secrets.AWS_CACHE_ROLE_NAME }} | ||
| region: ${{ secrets.AWS_CACHE_REGION }} | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm i | ||
|
|
||
| - name: Enforce module boundaries | ||
| run: pnpm exec nx run enforce-boundaries:lint:boundaries | ||
|
ysitbon marked this conversation as resolved.
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| "use strict"; | ||
|
|
||
| /** | ||
| * Module-boundary dependency constraints for the new architecture | ||
| * (domain/, shared/, features/). Shape mirrors @nx/enforce-module-boundaries | ||
| * depConstraints so this config ports verbatim to .oxlintrc.json when | ||
| * @nx/oxlint publishes stable. | ||
| * | ||
| * A rule fires only when the source package has the sourceTag. Legacy | ||
| * packages without matching tags (libs/, apps/, e2e/, tools/) are | ||
| * unconstrained on purpose. | ||
| */ | ||
| const DEP_CONSTRAINTS = [ | ||
| { sourceTag: "scope:shared", onlyDependOnLibsWithTags: ["scope:shared"] }, | ||
| { sourceTag: "scope:domain", onlyDependOnLibsWithTags: ["scope:domain", "scope:shared"] }, | ||
| { | ||
| sourceTag: "scope:features", | ||
| onlyDependOnLibsWithTags: ["scope:features", "scope:domain", "scope:shared"], | ||
| }, | ||
| { | ||
| sourceTag: "type:domain-entity", | ||
| onlyDependOnLibsWithTags: ["type:domain-entity", "scope:shared"], | ||
| }, | ||
| { | ||
| sourceTag: "type:domain-api", | ||
| onlyDependOnLibsWithTags: ["type:domain-entity", "type:domain-api", "scope:shared"], | ||
|
ysitbon marked this conversation as resolved.
|
||
| }, | ||
| ]; | ||
|
|
||
| module.exports = { DEP_CONSTRAINTS }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| { | ||
| "$schema": "../../../node_modules/nx/schemas/project-schema.json", | ||
| "name": "enforce-boundaries", | ||
| "projectType": "library", | ||
| "targets": { | ||
| "lint:boundaries": { | ||
| "executor": "nx:run-commands", | ||
| "cache": true, | ||
| "inputs": [ | ||
| "{workspaceRoot}/tools/nx-plugins/enforce-boundaries/**/*", | ||
| "{workspaceRoot}/tools/nx-plugins/project-tags/plugin.js", | ||
| "{workspaceRoot}/domain/**/package.json", | ||
| "{workspaceRoot}/shared/**/package.json", | ||
| "{workspaceRoot}/features/**/package.json", | ||
| "{workspaceRoot}/apps/**/package.json", | ||
| "{workspaceRoot}/libs/**/package.json", | ||
| "{workspaceRoot}/pnpm-workspace.yaml", | ||
|
ysitbon marked this conversation as resolved.
|
||
| "{workspaceRoot}/nx.cache-config.json" | ||
| ], | ||
| "options": { | ||
| "cwd": "{workspaceRoot}", | ||
| "command": "node tools/nx-plugins/enforce-boundaries/validate.js" | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
LL782 marked this conversation as resolved.
|
Binary file not shown.
|
ysitbon marked this conversation as resolved.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,210 @@ | ||
| "use strict"; | ||
|
|
||
| const test = require("node:test"); | ||
| const assert = require("node:assert/strict"); | ||
| const { findViolations } = require("./validate"); | ||
|
|
||
| /** | ||
| * @param {Array<{name: string, tags: string[]}>} projects | ||
| * @param {Array<{source: string, target: string}>} edges | ||
| */ | ||
| function buildGraph(projects, edges) { | ||
| const nodes = Object.fromEntries(projects.map(p => [p.name, { data: { tags: p.tags } }])); | ||
| const dependencies = Object.fromEntries( | ||
| projects.map(p => [ | ||
| p.name, | ||
| edges.filter(e => e.source === p.name).map(e => ({ target: e.target })), | ||
| ]), | ||
| ); | ||
| return { nodes, dependencies }; | ||
| } | ||
|
|
||
| test("shared -> shared is allowed", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:shared"] }, | ||
| { name: "b", tags: ["scope:shared"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); | ||
|
|
||
| test("shared -> domain is forbidden (leaf-layer rule)", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:shared"] }, | ||
| { name: "b", tags: ["scope:domain", "type:domain-entity"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| const v = findViolations(graph); | ||
| assert.equal(v.length, 1); | ||
| assert.equal(v[0].sourceName, "a"); | ||
| assert.deepEqual(v[0].sourceTags, ["scope:shared"]); | ||
| assert.equal(v[0].target, "b"); | ||
| }); | ||
|
|
||
| test("domain -> shared is allowed", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:domain", "type:domain-api"] }, | ||
| { name: "b", tags: ["scope:shared"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); | ||
|
|
||
| test("domain-entity -> domain-api is forbidden (intra-domain rule)", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:domain", "type:domain-entity"] }, | ||
| { name: "b", tags: ["scope:domain", "type:domain-api"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| const v = findViolations(graph); | ||
| assert.equal(v.length, 1); | ||
| assert.deepEqual(v[0].sourceTags, ["type:domain-entity"]); | ||
| assert.equal(v[0].target, "b"); | ||
| }); | ||
|
|
||
| test("domain-api -> domain-entity is allowed", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:domain", "type:domain-api"] }, | ||
| { name: "b", tags: ["scope:domain", "type:domain-entity"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); | ||
|
|
||
| test("domain-entity -> domain-entity is allowed", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:domain", "type:domain-entity"] }, | ||
| { name: "b", tags: ["scope:domain", "type:domain-entity"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); | ||
|
|
||
| test("features -> domain is allowed", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:features"] }, | ||
| { name: "b", tags: ["scope:domain", "type:domain-api"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); | ||
|
|
||
| test("features -> shared is allowed", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:features"] }, | ||
| { name: "b", tags: ["scope:shared"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); | ||
|
|
||
| test("features -> legacy libs is forbidden (features must stay in new arch)", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:features"] }, | ||
| { name: "b", tags: ["scope:libs", "scope:libs-non-ui"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| const v = findViolations(graph); | ||
| assert.equal(v.length, 1); | ||
| assert.deepEqual(v[0].sourceTags, ["scope:features"]); | ||
| }); | ||
|
|
||
| test("legacy source (no new-arch tags) is unconstrained", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:libs", "scope:libs-non-ui"] }, | ||
| { name: "b", tags: ["scope:domain", "type:domain-entity"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); | ||
|
|
||
| test("app source is unconstrained (apps can import anything during migration)", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "app", tags: ["scope:apps", "type:app-desktop"] }, | ||
| { name: "b", tags: ["scope:domain", "type:domain-entity"] }, | ||
| { name: "c", tags: ["scope:features"] }, | ||
| ], | ||
| [ | ||
| { source: "app", target: "b" }, | ||
| { source: "app", target: "c" }, | ||
| ], | ||
| ); | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); | ||
|
|
||
| test("external deps (target absent from graph.nodes) are skipped", () => { | ||
| const graph = { | ||
| nodes: { a: { data: { tags: ["scope:shared"] } } }, | ||
| dependencies: { a: [{ target: "npm:some-package" }, { target: "npm:another" }] }, | ||
| }; | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); | ||
|
|
||
| test("multiple violations are all reported", () => { | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:shared"] }, | ||
| { name: "b", tags: ["scope:domain"] }, | ||
| { name: "c", tags: ["scope:features"] }, | ||
| ], | ||
| [ | ||
| { source: "a", target: "b" }, | ||
| { source: "a", target: "c" }, | ||
| ], | ||
| ); | ||
| const v = findViolations(graph); | ||
| assert.equal(v.length, 2); | ||
| assert.ok(v.every(x => x.sourceName === "a" && x.sourceTags.includes("scope:shared"))); | ||
| }); | ||
|
|
||
| test("multi-tag source emits one deduped violation per edge", () => { | ||
| // type:domain-api source also carries scope:domain — both rules match, | ||
| // both forbid scope:features targets. Should produce ONE violation | ||
| // listing both source tags. | ||
| const graph = buildGraph( | ||
| [ | ||
| { name: "a", tags: ["scope:domain", "type:domain-api"] }, | ||
| { name: "b", tags: ["scope:features"] }, | ||
| ], | ||
| [{ source: "a", target: "b" }], | ||
| ); | ||
| const v = findViolations(graph); | ||
| assert.equal(v.length, 1); | ||
| assert.equal(v[0].sourceName, "a"); | ||
| assert.equal(v[0].target, "b"); | ||
| assert.ok(v[0].sourceTags.includes("scope:domain")); | ||
| assert.ok(v[0].sourceTags.includes("type:domain-api")); | ||
| }); | ||
|
|
||
| test("missing tags default to empty array (no crash)", () => { | ||
| const graph = { | ||
| nodes: { | ||
| a: { data: {} }, | ||
| b: {}, | ||
| }, | ||
| dependencies: { a: [{ target: "b" }] }, | ||
| }; | ||
| // neither node carries tags; no rule fires | ||
| assert.deepEqual(findViolations(graph), []); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.