tbonelee commented on PR #5257:
URL: https://github.com/apache/zeppelin/pull/5257#issuecomment-4534363888

   **praise:** Lint is clean now. `Chart | null` / `ChartConfiguration | null` 
resolves both `no-explicit-any` errors, and the union still narrows per-case at 
the `{ type: 'bar', ... }` literal assignment, so each case's dataset/options 
shape stays type-checked. Verified locally with the project ruleset: errors 4 → 
0 on this file.
   
   **thought (non-blocking):** If you ever want tighter per-case typing, 
`Chart<'bar'>` / `ChartConfiguration<'bar'>` etc. drop the union entirely and 
narrow without relying on the assignment site. Current shape is good; 
completely optional follow-up.
   
   **nitpick (non-blocking):** Now that `container` is captured at effect 
start, the `!container` check inside `import('chart.js/auto').then(...)` and 
the `if (container)` guard in the cleanup are unreachable. `container` was 
already non-null past the early return, and closure capture pins it for the 
effect's lifetime. Safe to simplify to `if (cancelled) return;` and 
`container.innerHTML = '';`. Pure tidy-up, no behavior change.
   


-- 
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]

Reply via email to