diff --git a/.github/workflows/build-and-test-pr.yml b/.github/workflows/build-and-test-pr.yml index f8fe3bf5661c..c73b8a3ee2ac 100644 --- a/.github/workflows/build-and-test-pr.yml +++ b/.github/workflows/build-and-test-pr.yml @@ -94,6 +94,13 @@ jobs: uses: LedgerHQ/ledger-live/.github/workflows/test-features-reusable.yml@develop secrets: inherit + enforce-boundaries: + name: "Enforce Module Boundaries" + needs: determine-affected + if: ${{(contains(needs.determine-affected.outputs.paths, 'domain') || contains(needs.determine-affected.outputs.paths, 'shared') || contains(needs.determine-affected.outputs.paths, 'features')) && !github.event.pull_request.head.repo.fork}} + uses: LedgerHQ/ledger-live/.github/workflows/test-boundaries-reusable.yml@develop + secrets: inherit + test-design-system: name: "Test UI Libs" needs: determine-affected diff --git a/.github/workflows/test-boundaries-reusable.yml b/.github/workflows/test-boundaries-reusable.yml new file mode 100644 index 000000000000..e211eaf95490 --- /dev/null +++ b/.github/workflows/test-boundaries-reusable.yml @@ -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 diff --git a/package.json b/package.json index a9af4f7da5a6..bca213cb7f6b 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "scripts": { "nx:write-cache-config": "node tools/nx/write-nx-cache-config.mjs", "postinstall": "pnpm run nx:write-cache-config", + "lint:boundaries": "nx run enforce-boundaries:lint:boundaries", "bump": "changeset version", "clean": "git clean -fdX", "changelog": "changeset add", diff --git a/tools/nx-plugins/enforce-boundaries/constraints.js b/tools/nx-plugins/enforce-boundaries/constraints.js new file mode 100644 index 000000000000..b477cb25f33f --- /dev/null +++ b/tools/nx-plugins/enforce-boundaries/constraints.js @@ -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"], + }, +]; + +module.exports = { DEP_CONSTRAINTS }; diff --git a/tools/nx-plugins/enforce-boundaries/project.json b/tools/nx-plugins/enforce-boundaries/project.json new file mode 100644 index 000000000000..cdccbadac398 --- /dev/null +++ b/tools/nx-plugins/enforce-boundaries/project.json @@ -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", + "{workspaceRoot}/nx.cache-config.json" + ], + "options": { + "cwd": "{workspaceRoot}", + "command": "node tools/nx-plugins/enforce-boundaries/validate.js" + } + } + } +} diff --git a/tools/nx-plugins/enforce-boundaries/validate.js b/tools/nx-plugins/enforce-boundaries/validate.js new file mode 100644 index 000000000000..c996f6224347 Binary files /dev/null and b/tools/nx-plugins/enforce-boundaries/validate.js differ diff --git a/tools/nx-plugins/enforce-boundaries/validate.test.js b/tools/nx-plugins/enforce-boundaries/validate.test.js new file mode 100644 index 000000000000..c659ff79259d --- /dev/null +++ b/tools/nx-plugins/enforce-boundaries/validate.test.js @@ -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), []); +}); diff --git a/tools/nx-plugins/project-tags/plugin.js b/tools/nx-plugins/project-tags/plugin.js index c7ca49b0aa54..5de8be1ebde5 100644 --- a/tools/nx-plugins/project-tags/plugin.js +++ b/tools/nx-plugins/project-tags/plugin.js @@ -33,6 +33,19 @@ function inferTags(projectRoot, packageName) { tags.add("scope:features"); } + if (projectRoot.startsWith("domain/")) { + tags.add("scope:domain"); + if (projectRoot.startsWith("domain/entity/")) { + tags.add("type:domain-entity"); + } else if (projectRoot.startsWith("domain/api/")) { + tags.add("type:domain-api"); + } + } + + if (projectRoot.startsWith("shared/")) { + tags.add("scope:shared"); + } + if (projectRoot.startsWith("e2e/")) { tags.add("scope:e2e"); } diff --git a/tools/nx-plugins/project-tags/plugin.test.js b/tools/nx-plugins/project-tags/plugin.test.js new file mode 100644 index 000000000000..856627283623 --- /dev/null +++ b/tools/nx-plugins/project-tags/plugin.test.js @@ -0,0 +1,76 @@ +"use strict"; + +const test = require("node:test"); +const assert = require("node:assert/strict"); +const { inferTags } = require("./plugin"); + +test("domain/entity/* gets scope:domain + type:domain-entity", () => { + assert.deepEqual(inferTags("domain/entity/currency", "@domain/entity-currency"), [ + "scope:domain", + "scope:no-apps", + "type:domain-entity", + ]); + assert.deepEqual(inferTags("domain/entity/currency-fiat", "@domain/entity-currency-fiat"), [ + "scope:domain", + "scope:no-apps", + "type:domain-entity", + ]); +}); + +test("domain/api/* gets scope:domain + type:domain-api", () => { + assert.deepEqual(inferTags("domain/api/foo", "@domain/api-foo"), [ + "scope:domain", + "scope:no-apps", + "type:domain-api", + ]); +}); + +test("shared/* gets scope:shared", () => { + assert.deepEqual(inferTags("shared/feature-flags", "@shared/feature-flags"), [ + "scope:no-apps", + "scope:shared", + ]); + assert.deepEqual(inferTags("shared/schema-primitives", "@shared/schema-primitives"), [ + "scope:no-apps", + "scope:shared", + ]); +}); + +test("features/* gets scope:features (regression)", () => { + assert.deepEqual(inferTags("features/market-banner", "@features/market-banner"), [ + "scope:features", + "scope:no-apps", + ]); +}); + +test("libs/* does not pick up new-arch tags (regression)", () => { + const libTags = inferTags("libs/ledger-live-common", "@ledgerhq/live-common"); + assert.ok(!libTags.includes("scope:domain"), `got ${libTags.join(",")}`); + assert.ok(!libTags.includes("scope:shared")); + assert.ok(!libTags.includes("type:domain-entity")); + assert.ok(libTags.includes("scope:libs")); + assert.ok(libTags.includes("type:live-common")); +}); + +test("apps/* still get scope:apps and not scope:no-apps (regression)", () => { + const desktop = inferTags("apps/ledger-live-desktop", "ledger-live-desktop"); + assert.ok(desktop.includes("scope:apps")); + assert.ok(desktop.includes("type:app-desktop")); + assert.ok(!desktop.includes("scope:no-apps")); + assert.ok(!desktop.includes("scope:domain")); +}); + +test("libs/ui/* sub-scoping still works (regression)", () => { + assert.deepEqual(inferTags("libs/ui/packages/react", "@ledgerhq/ui-react"), [ + "scope:libs", + "scope:libs-ui", + "scope:no-apps", + ]); +}); + +test("domain path that is not entity/ or api/ still gets scope:domain only", () => { + assert.deepEqual(inferTags("domain/other/thing", "@domain/other-thing"), [ + "scope:domain", + "scope:no-apps", + ]); +});