fix(lineage): use namespace import for @dagrejs/dagre to avoid undefined graphlib#29949
fix(lineage): use namespace import for @dagrejs/dagre to avoid undefined graphlib#29949SAY-5 wants to merge 1 commit into
Conversation
…ned graphlib Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review ✅ ApprovedSwitches to a namespace import for OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| */ | ||
|
|
||
| import { graphlib, layout } from '@dagrejs/dagre'; | ||
| import * as dagre from '@dagrejs/dagre'; |
There was a problem hiding this comment.
If Vite or Jest resolves @dagrejs/dagre through its ESM entry instead of the CommonJS wrapper, import * as dagre can produce a namespace where the module object is under default. In that case the changed dagre.graphlib.Graph and dagre.layout reads become undefined, so opening lineage still crashes instead of fixing the interop path. The sibling working pattern uses a default import for this module object.
Context Used: CLAUDE.md (source)
harshach
added
the
safe to test
Jest test CoverageUI tests summary
|
|
🟡 Playwright Results — all passed (26 flaky)✅ 4464 passed · ❌ 0 failed · 🟡 26 flaky · ⏭️ 129 skipped
🟡 26 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
2 participants
Fixes #29556.
EntityLineageLayoutUtils.tsimports@dagrejs/dagrewith named imports (import { graphlib, layout }), but that package is CommonJS and assigns everything throughmodule.exports = { graphlib, layout, ... }. Under the dev bundler's CJS/ESM interop the namedgraphlibbinding can resolve toundefined, sographlib.GraphthrowsCannot read properties of undefined (reading 'Graph')when the lineage layout runs. The siblingWorkflowLayout.tsalready imports the module as a whole and readsdagre.graphlib.Graph, which is why it does not hit this.This switches the file to a namespace import (
import * as dagre) and accessesdagre.graphlib.Graph/dagre.layout, matching the workingWorkflowLayout.tsand binding the full module object regardless of how the bundler handles the CJS named exports. I confirmed the pattern type-checks with the project's tsconfig flags and thatdagre.graphlib.Graph/dagre.layoutare defined at runtime; I could not reproduce the exact dev-server interop failure locally, so I mirrored the already-working import style rather than guessing at a bundler config change.Greptile Summary
This PR changes how the lineage layout utility reads Dagre. The main changes are:
graphlib.Graphfrom the Dagre module object.layoutthrough the Dagre module object.Confidence Score: 5/5
This looks safe to merge after checking the Dagre module shape used by the app bundler.
default.openmetadata-ui/src/main/resources/ui/src/utils/EntityLineageLayoutUtils.ts
Important Files Changed
Reviews (1): Last reviewed commit: "fix(lineage): use namespace import for @..." | Re-trigger Greptile
Context used (3)