Raja-Hamid opened a new pull request, #7512:
URL: https://github.com/apache/texera/pull/7512
### What changes were proposed in this PR?
Removes two pieces of unreachable code, each surfaced by a surviving
mutation — i.e. each was *demonstrated* to have no observable effect, not
merely suspected. No user-facing behavior changes, hence `refactor`.
**1. `hugging-face-image-upload.component.html` — an unreachable fallback**
```diff
-<span>{{ displayFileName || "Selected image" }}</span>
+<span>{{ displayFileName }}</span>
The enclosing block is *ngIf="previewSrc", and previewSrc returns
this.hasImage ? this.formControl.value : "" — so it is non-empty exactly when
hasImage is true. displayFileName returns fileName or, failing that, "Uploaded
image" whenever hasImage is true. It is therefore never empty where this
renders, and the right-hand side of the || can never be evaluated.
2. drag-drop.service.ts — a guard subsumed by the next one
const jointLink = paper.getModelById(link.linkID) as joint.dia.Link;
-if (!jointLink) {
- continue;
-}
-
const linkView = paper.findViewByModel(jointLink) as joint.dia.LinkView;
if (!linkView) {
continue;
}
findViewByModel in jointjs 3.5.4 resolves its argument with (isString(cell)
|| isNumber(cell)) ? cell : (cell && cell.id) and returns this._views[id]. The
cell && cell.id short-circuits on undefined, so the call returns undefined
rather than throwing, and the !linkView guard already catches everything
!jointLink did.
Worth noting for reviewers: this guard was not meaningless.
findIntersectedLink iterates the Texera graph but resolves each link against
the JointJS paper, and those two can genuinely diverge, so getModelById really
can return undefined at runtime. That path is still handled, just one guard
later. (The as joint.dia.Link cast is what hides the undefined case from the
type checker.)
No screenshots: neither change alters rendered output or behavior, that is
precisely the claim under test.
Any related issues, documentation, discussions?
Closes #7459
How was this PR tested?
Because this is a refactor with no behavior change, the removed branches
cannot be driven red by definition. Instead, two regression guards were added
that pass both before and after the removals, pinning the behavior the removals
must preserve:
- hugging-face-image-upload.component.spec.ts, an image with no filename
renders exactly Uploaded image; a known filename renders instead; and with no
image the preview block does not render at all, so the label is never empty.
- drag-drop.service.spec.ts, a link present in the Texera graph but with no
model in the JointJS paper is skipped: findIntersectedLink returns null and
does not throw. (Removing both guards makes this fail on linkView.el, so it
genuinely pins the path.)
corepack yarn ng test --watch=false \
--include="**/hugging-face-image-upload.component.spec.ts" \
--include="**/drag-drop.service.spec.ts"
- Against unmodified source: 2 files, 56/56 passing — the new guards are
green before any code is touched.
- After both removals: 2 files, 56/56 passing, with no assertion edits.
Wider sweep, adding the specs for the formly config that registers the
component:
corepack yarn ng test --watch=false \
--include="**/formly*.spec.ts" \
--include="**/hugging-face-image-upload.component.spec.ts" \
--include="**/drag-drop.service.spec.ts"
→ 4 files, 83/83 passing
yarn prettier-eslint reports all four touched files unchanged.
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Yes alongside Claude Code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
