fix-cyclic-deps

name

fix-cyclic-deps

description

Detect and resolve cyclic module dependencies introduced during UI5 modernization. Trigger when user mentions: "cyclic dependency", "circular import", "undefined module at runtime", "lazy require", "sap.ui.require sync", "import cycle", or when a module returns undefined despite a correct import path. Classic symptom: a module is `undefined` at runtime despite a correct sap.ui.define import path. Auto-fixes 2-node cycles (A↔B) by converting the lesser-used edge to lazy sap.ui.require(). Auto-fixes longer chains (3+ nodes) via hub-based approach. Reports unfixable chains to MODERNIZATION-ISSUES.md. Runs as Phase 3, Step 3.3 (final step) after all other Phase 3 steps.

Fix Cyclic Module Dependencies

This skill detects and resolves cyclic module dependencies that arise during UI5 modernization. When modernization converts global namespace access to sap.ui.define imports, new dependency edges can create circular imports (A imports B, B imports A), causing the UI5 AMD loader to return undefined for the back-edge module.

The fix: replace the back-edge sap.ui.define dependency with a lazy sap.ui.require("path/to/Module") (synchronous form) at each call site, retrieving the already-loaded module from the loader cache without creating a dependency edge.

Linter Rule

Rule ID	Message Pattern	This Skill's Action
(none — structural)	Runtime: module is undefined despite correct import path	Detect cycle in dependency graph, convert back-edge to lazy sap.ui.require()

This skill is NOT triggered by a UI5 linter rule. It addresses a structural problem in the module dependency graph. It is triggered as the final fix phase in the modernization workflow, or standalone when a developer encounters undefined modules at runtime.

When to Use

• In modernization workflow: As Phase 3, Step 3.3 (final step) after ALL other Phase 3 steps complete. Multiple steps add sap.ui.define edges that can create cycles — running once at the end operates on the final dependency graph.
• Standalone: When a module returns undefined at runtime despite a correct import path.
• After manual refactoring: When a developer adds a new sap.ui.define dependency and gets an unexpected undefined.

Sources of Cycles

Three modernization operations add new sap.ui.define dependency edges that can introduce cycles:

1. fix-js-globals Case 1c — Global Namespace Reads

Converting var Helper = com.example.app.utils.Helper; to a sap.ui.define dependency. If Helper already imports the current module, adding the reverse edge creates a 2-node cycle.

2. fix-js-globals Case 10 — jQuery.sap.declare/require Modernization

Wrapping legacy jQuery.sap.declare / jQuery.sap.require code in sap.ui.define. The jQuery.sap.require calls become dependencies, potentially creating cycles the legacy synchronous loader handled differently.

3. modernize-test-starter — Test File Dependencies

Test files that previously accessed modules via the global namespace chain now get proper sap.ui.define imports. Test utility files that reference each other can create cycles.

Background — Why Cycles Break the UI5 Loader

The UI5 AMD loader resolves sap.ui.define dependencies via depth-first traversal. When it encounters a cycle:

1. Loader starts loading Module A
2. A depends on B → loader starts loading B
3. B depends on A → but A isn't finished yet
4. Loader returns undefined for A's factory result (the back-edge)
5. B's factory receives undefined where it expected A's exports

2-node cycles (A↔B) are guaranteed to break. Longer chains (A→B→C→A) may or may not break at runtime depending on load order — they are latent bugs.

Why sap.ui.require (synchronous) Breaks the Cycle

The synchronous form sap.ui.require("path/to/Module") does NOT create a loader dependency edge. It reads from the module cache without triggering a load.

Critical caveat: sap.ui.require(path) returns undefined if the target module's factory has not yet executed. So it is safe only when the target module is reachable via a static-only path from the active entry point that fires before the lazy call site.

When you remove a static edge A → B and replace it with var B = sap.ui.require("B") inside a function in A, you must verify B remains reachable from every entry-point controller that loads A.

Three forms of sap.ui.require — know the difference:

• sap.ui.require("path/to/B") — sync cache read. No load trigger. Returns undefined if not in cache.
• sap.ui.require(["path/to/B"], function(B) {...}) — async load + callback. Triggers load.
• sap.ui.requireSync("path/to/B") — sync load + return. Triggers load. Deprecated.

Key distinction:

• sap.ui.define(["path/to/B"], ...) — creates a loader edge A→B (cycle risk)
• var B = sap.ui.require("path/to/B") inside a function body — no loader edge (safe if B is in cache)

Detection Algorithm

Automated Detection Script

A bundled script automates detection (graph building, verification, cycle detection, usage counting, hub identification):

node <skill-dir>/scripts/detect-cycles.js <project-root>

The script:

1. Discovers the project namespace from manifest.json
2. Scans all .js files (app + tests), strips comments, parses sap.ui.define arrays
3. Builds the dependency graph (internal project modules only)
4. Verifies graph completeness with fallback analysis
5. Detects 2-node cycles and longer chains (Tarjan's SCC)
6. Counts usages and identifies lazy side (2-node) or hub (3+ node)

Output is JSON to stdout. Use this to drive the fix phase. If unavailable, the manual procedure is below and in references/dependency-graph-analysis.md.

Manual Procedure

Step 1 — Discover Project Namespace

Read manifest.json → sap.app.id → e.g. "com.example.myapp" → convert to slash notation: "com/example/myapp". This identifies internal project modules vs. sap/* framework deps.

Step 2 — Build Dependency Graph

Parse all .js files (app source AND tests). For each file:

1. Strip comments (// and /* */) while preserving string contents. Prevents matching sap.ui.define in commented-out code.

2. Find sap.ui.define using a regex allowing arbitrary whitespace between tokens:

   /sap\s*\.\s*ui\s*\.\s*define\s*\(/
   

   Critical: The simpler sap\.ui\.define\s*\( MISSES files where sap.ui.define is split across lines. These patterns exist in real codebases.

3. Extract the dependency array: From ( after define, find [ ... ] respecting bracket depth. Extract string literals.

4. Filter to internal project modules only (matching namespace). Include test-resources/-prefixed paths.

5. Build directed graph: node = module path (without .js), edge = dependency.

Exclusions: Framework deps (sap/*), sap.ui.require calls in function bodies, string literals (extend names, fragment paths).

Step 2b — Verify Graph Completeness

After building the graph, verify every referenced dependency was also parsed. A missing module can hide entire cycle chains.

For every module referenced as a dependency but having no graph node entry:

1. Check if source file exists. If not, skip with info note.
2. If file exists, run fallback analysis: scan for internal namespace strings inside [...] arrays. If found but primary parser extracted nothing, merge into graph.
3. Also check files where sap.ui.define was found but first argument was not an array. Run same fallback.

Step 3 — Detect 2-Node Cycles

For every edge A→B, check if B→A also exists. Collect unique pairs (deduplicated).

Step 4 — Detect Longer Chains (3+ Nodes)

Run Tarjan's SCC algorithm. Any SCC with 3+ nodes is a longer cycle chain. See references/dependency-graph-analysis.md for full algorithm pseudocode.

Fix Strategy for 2-Node Cycles

Decision: Which Side Gets Lazy Treatment

Phase 1 — Runtime Reachability Check

1. Identify entry points: Component.js + every controller that statically imports A or B.
2. For each entry point, walk static dep graph and check which of {A, B} is reachable.
3. Choose lazy side based on coverage:
   • If A reachable from every entry point using B → safe to make A lazy in B.
   • If B reachable from every entry point using A → safe to make B lazy in A.
   • If both have full coverage → proceed to Phase 2.
   • If NEITHER has full coverage → requires also adding a static dep to controllers (see "Append-to-Controller Remedy").

Phase 2 — Usage Count Tiebreaker

When both sides are equally safe:

1. Count usages: References to B in A's code body vs. references to A in B's code body (exclude strings, comments, sap.ui.define array).
2. Fewer usages wins: Less code churn.
3. Tiebreaker 1: Module with more total deps keeps normal import (orchestrator pattern).
4. Tiebreaker 2: Alphabetically first keeps normal import.

Transformation Steps

To make B lazy in A (A no longer statically imports B):

Step 1 — Remove B from A's dependency array and corresponding parameter:

// Before:
sap.ui.define(["path/to/B", "path/to/C"], function(B, C) {
// After:
sap.ui.define(["path/to/C"], function(C) {

Step 2 — Add lazy require at each usage site:

// Before:
someMethod: function() {
    B.doSomething();
    B.doSomethingElse();
}
// After:
someMethod: function() {
    var B = sap.ui.require("path/to/B");
    B.doSomething();
    B.doSomethingElse();
}

One sap.ui.require per function — each function using B needs its own declaration.

Declaration keyword: Match surrounding style (const for modern, var for legacy).

Step 3 — Clean up: Remove var X = X; self-assignments. If B had zero code references, just remove from deps — no lazy require needed.

Special Case: Module Already Partially Lazy

If A already has sap.ui.require("path/to/B") calls AND B in its sap.ui.define array, remove B from sap.ui.define and ensure all remaining usage sites have lazy requires. Don't duplicate existing calls.

Longer Chains (3+ Nodes) — Hub-Based Auto-Fix

Longer chains are auto-fixed using a hub-based approach. Identify the hub module within each SCC and make its cycle-creating dependencies lazy. One hub fix can eliminate many chains simultaneously.

Why Hub-Based?

A hub module sits at the center of multiple cycle paths. Rather than fixing edges in many files, converting a few deps in the hub breaks all cycles at once.

Hub Identification Algorithm

For each SCC with 3+ nodes:

1. Cycle participation score per node: count outgoing edges to other SCC members. Higher = more cycles broken by making those deps lazy.
2. Select hub: Highest score. Tiebreaker: more total deps (orchestrator). Final: alphabetical.
3. Identify cycle-creating deps: Hub's sap.ui.define deps that are SCC members.
4. Apply same transformation as 2-node cycles.
5. Re-run SCC detection: Repeat if cycles remain.

Decision: Which Hub Deps to Make Lazy

Make all SCC-internal deps of the hub lazy. This guarantees all cycles through the hub are broken. If a hub's SCC-internal dep has zero usages, just remove it.

Fallback: Report to MODERNIZATION-ISSUES.md

If a module references the cycle-creating dep at top level outside any function body (where lazy sap.ui.require would return undefined), report:

### Cyclic Dependency Chain (unfixable automatically)

- **SCC nodes**: A, B, C, D
- **Hub identified**: A (3 outgoing SCC edges, 4 incoming)
- **Blocking reason**: A references B at module top level (line 15), outside any function body.
- **Suggested manual fix**: Restructure A to defer the B reference into a function body, or extract the top-level initialization into a separate non-cyclic module.

Before/After Examples

Example 1 — 2-Node Cycle with 1 Usage (ModuleA ↔ ModuleB)

ModuleA imports ModuleB, ModuleB imports ModuleA. ModuleB only uses ModuleA at 1 call site.

Before (broken):

// ModuleB.js
sap.ui.define([
    "com/example/myapp/utils/ModuleA",
    "sap/ui/thirdparty/jquery"
], function(ModuleA, jQuery) {
    var ModuleB = {
        handleStatus: function(aSelectedItems, sId) {
            ModuleA.processStatus(aSelectedItems, sId);  // undefined!
        }
    };
    return ModuleB;
});

After (fixed):

// ModuleB.js
sap.ui.define([
    "sap/ui/thirdparty/jquery"
], function(jQuery) {
    var ModuleB = {
        handleStatus: function(aSelectedItems, sId) {
            var ModuleA = sap.ui.require("com/example/myapp/utils/ModuleA");
            ModuleA.processStatus(aSelectedItems, sId);
        }
    };
    return ModuleB;
});

Example 2 — 2-Node Cycle with Multiple Usages (Orchestrator ↔ Helper)

Orchestrator imports Helper (5 usages), Helper imports Orchestrator (25 usages). Decision: make Helper lazy in Orchestrator (fewer sites to patch).

Before (broken):

// Orchestrator.js
sap.ui.define([
    "com/example/myapp/utils/Helper",
    "com/example/myapp/utils/Validator"
], function(Helper, Validator) {
    var Orchestrator = {
        openDialog: function() { Helper.openDialog(); },
        refreshAll: function() { Helper.refreshAll(); }
        // ... 3 more Helper usage sites
    };
    return Orchestrator;
});

After (fixed):

// Orchestrator.js
sap.ui.define([
    "com/example/myapp/utils/Validator"
], function(Validator) {
    var Orchestrator = {
        openDialog: function() {
            var Helper = sap.ui.require("com/example/myapp/utils/Helper");
            Helper.openDialog();
        },
        refreshAll: function() {
            var Helper = sap.ui.require("com/example/myapp/utils/Helper");
            Helper.refreshAll();
        }
        // ... each function gets its own lazy require
    };
    return Orchestrator;
});

Example 3 — Unused Dependency Removal

ModuleX imports ModuleY but never references it in code — just remove the dead import.

Before:

sap.ui.define([
    "com/example/myapp/utils/ModuleY",
    "com/example/myapp/utils/ModuleZ"
], function(ModuleY, ModuleZ) {
    // ModuleY never used

After:

sap.ui.define([
    "com/example/myapp/utils/ModuleZ"
], function(ModuleZ) {

Example 4 — Longer Chain Auto-Fixed via Hub (ModelManager Hub)

ModelManager participates in 7 cycle chains through deps FilterHelper and ChartHelper. Hub analysis: ModelManager has 2 outgoing SCC edges — it is the hub.

Before (broken):

// ModelManager.js
sap.ui.define(["sap/ui/thirdparty/jquery",
    "com/example/myapp/utils/FilterHelper",
    "com/example/myapp/utils/Payload",
    "com/example/myapp/utils/ChartHelper",
    "sap/base/Log"
], function(jQuery, FilterHelper, oPayload, ChartHelper, Log) {
    var ModelManager = {
        getFilterConfig: function() {
            var oFilterConfig = FilterHelper;  // undefined due to cycle!
        },
        getChartType: function() {
            var oChartType = ChartHelper.getAnnotationType();  // undefined!
        }
    };
    return ModelManager;
});

After (fixed — both SCC-internal deps made lazy):

// ModelManager.js
sap.ui.define(["sap/ui/thirdparty/jquery",
    "com/example/myapp/utils/Payload",
    "sap/base/Log"
], function(jQuery, oPayload, Log) {
    var ModelManager = {
        getFilterConfig: function() {
            var FilterHelper = sap.ui.require("com/example/myapp/utils/FilterHelper");
            var oFilterConfig = FilterHelper;
        },
        getChartType: function() {
            var ChartHelper = sap.ui.require("com/example/myapp/utils/ChartHelper");
            var oChartType = ChartHelper.getAnnotationType();
        }
    };
    return ModelManager;
});

Removing 2 deps from 1 file broke all 7 cycle chains. Note: var oFilterConfig = FilterHelper; is a legacy aliasing pattern — the skill preserves existing code, only transforming the import mechanism.

Implementation Steps

1. Run detection script to build the dependency graph and detect all cycles:

   node <skill-dir>/scripts/detect-cycles.js <project-root>
   

   JSON output contains twoNodeCycles (with usage counts and lazy-side decisions) and longerChains (with hub identification). If unavailable, follow the manual procedure above.

2. Review output: Check warnings and errors arrays for graph completeness issues. Address errors before proceeding.

3. Fix each 2-node cycle (from twoNodeCycles[]):

   • lazySide field identifies which module to patch
   • Remove cyclic dep from sap.ui.define array + function parameter
   • Add var Module = sap.ui.require("path/to/Module") at each usage site
   • Clean up artifacts

4. Fix longer chains via hub (from longerChains[]):

   • hub and hubInternalDeps fields identify module and deps to make lazy
   • If ALL usages are inside function bodies → apply lazy require transformation
   • If any usage at module top level → report to MODERNIZATION-ISSUES.md
   • Re-run SCC detection — repeat if cycles remain

5. Verify — re-run cycle detection:

   node <skill-dir>/scripts/detect-cycles.js <project-root>
   

   Output should show twoNodeCycles: [] and longerChains: []. Then run npx @ui5/linter --details for regression check.

6. Verify static coverage:

   node <skill-dir>/scripts/detect-unsafe-lazy.js <project-root>
   

   Checks every sap.ui.require("M") has M reachable via static chain from every entry-point controller. If findings exist, apply "Append-to-Controller Remedy" below. Re-run until unsafeCount: 0. Skill is not done until BOTH scripts report 0 issues.

Append-to-Controller Remedy

When detect-unsafe-lazy.js reports uncovered entry points, append the lazy target to the controller's sap.ui.define array as a load-only side-effect import:

// Before — controller does NOT statically import DialogHelper
sap.ui.define([
    "com/example/myapp/utils/ActionHandler",
    "sap/ui/core/mvc/Controller"
], function(ActionHandler, Controller) {
    // ActionHandler has lazy require to DialogHelper
    // → sap.ui.require("DialogHelper") returns undefined

// After — append as load-only dep (no factory param)
sap.ui.define([
    "com/example/myapp/utils/ActionHandler",
    "sap/ui/core/mvc/Controller",
    "com/example/myapp/utils/DialogHelper"
], function(ActionHandler, Controller /* no DialogHelper param */) {
    // DialogHelper now in cache → lazy require works

Rules:

• Append at end of dep array, no corresponding factory parameter
• If staticUncovered lists 3+ controllers, add dep to BaseController.js or Component.js instead
• Re-run detect-unsafe-lazy.js after each fix to confirm resolution

Notes — Critical Rules

1. Lazy require INSIDE function body: Place var B = sap.ui.require("path/to/B") inside the function that uses B, not at module top level. At define-time, B may not be in cache yet.

2. One require per function: Each function body using the module needs its own declaration. The variable is function-scoped.

3. Only internal project modules: Framework deps (sap/*) never cause project-level cycles. Only process deps matching the project namespace.

4. String literals are NOT usages: Log messages, .extend() class names, fragment paths are strings, not code references. Do not count or modify them.

5. Commented-out code is NOT a usage: Lines inside // or /* */ do not count as usages.

6. Parameter alignment: Removing dep at index N → remove function parameter at index N. Adjust only if the removed dep had a corresponding parameter.

7. Existing lazy requires are NOT dependency edges: sap.ui.require("path/to/X") in function bodies does NOT create loader edges. Only sap.ui.define array entries are edges.

8. Idempotent: Running twice is safe. On second run, cycles are already broken — no changes made.

9. Test files included: Dependency graph must include test files. They can participate in cycles after modernize-test-starter adds sap.ui.define deps.

10. Do not touch async sap.ui.require: The async form sap.ui.require(["path/to/B"], function(B) {...}) creates a loader edge. This skill only uses the synchronous single-string form.

11. Multi-line sap.ui.define patterns: Regex MUST allow arbitrary whitespace/newlines between sap, ., ui, ., define, (. Simpler regexes miss split patterns, hiding cycle chains.

12. Strip comments before parsing: Always remove comments (respecting string literals) before searching for sap.ui.define. Files may contain it in comments as section markers.

Related Skills

• fix-js-globals: Case 1c and case 10 are primary sources of new dependency edges creating cycles. Run fix-cyclic-deps AFTER fix-js-globals completes.
• modernize-test-starter: Test file modernization adds sap.ui.define dependencies that can create cycles between test utilities or between test and app modules.
• modernize-ui5-app: Parent workflow that orchestrates phase ordering. This skill runs as Phase 3, Step 3.3 (final step).