fix(extract): don't report deferred import() as a file cycle#1679
Open
Synvoya wants to merge 1 commit into
Open
fix(extract): don't report deferred import() as a file cycle#1679Synvoya wants to merge 1 commit into
Synvoya wants to merge 1 commit into
Conversation
…y-Labs#1241) `_dynamic_import_js` emitted a deferred `import('./x')` as a plain `imports_from` edge, so `find_import_cycles` counted it as a static import. A file that statically imports another which dynamically imports it back was reported as a phantom circular dependency. Keep the edge as `imports_from` (the dependency stays visible in the graph) but mark it `deferred`, and skip deferred edges in `find_import_cycles`. Closes Graphify-Labs#1241
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What it fixes
A deferred
import('./x')was emitted by_dynamic_import_js(graphify/extract.py) as a plainimports_fromedge — indistinguishable from a static import.find_import_cycles(graphify/analyze.py) counts everyimports_from/re_exportsedge as a hard file-cycle edge, so two files that reference each other via one static import + one dynamicimport()were reported as a circular dependency (#1241).Fix
Keep the deferred edge as
imports_from— the dependency stays visible in the graph — but tag it"deferred": True, and skip deferred edges infind_import_cycles.This is deliberately not a retag to
dynamic_import: the deferred edge's source is a symbol node (the enclosing function) and its target isn't a materialized file node, so the valid-id prune passes (extract.py:5380and the sibling passes) only spare it while its relation isimports_from/re_exports. Retagging would drop the edge entirely and lose the dependency; thedeferredmarker keeps it.find_import_cycles→[{'cycle': ['actions.ts', 'modal.ts'], 'length': 2, ...}][], and the deferred dependency edge is still present (imports_from,deferred=True).Test
tests/test_js_import_resolution.py::test_ts_dynamic_import_does_not_create_phantom_cycleasserts the deferred edge stays (imports_from+deferred), the real static import is unaffected, andfind_import_cyclesreturns no cycle. Fulltest_js_import_resolution.py+test_analyze.pypass (99 passed); reverting the fix fails the new test (negative control).Closes #1241