|
|
@@ -226,7 +226,6 @@ When a cell or range is selected, show a compact set of floating action buttons
|
|
|
|
|
|
## 12. Code Health & Refactoring
|
|
|
|
|
|
-> The items below were identified from a partial read of the codebase. The remaining files should be audited for similar issues before this section is considered complete.
|
|
|
|
|
|
### Bugs & Correctness
|
|
|
|
|
|
@@ -242,12 +241,24 @@ When a cell or range is selected, show a compact set of floating action buttons
|
|
|
|
|
|
The retry loop — `(recur data (concat (rest queue) (list cur)))` — re-queues cells with unsatisfied dependencies indefinitely. Cycle detection in `evaluate-from-cell` should prevent an infinite loop in practice, but there is no explicit guard. Add a visited set or iteration counter as a safety net.
|
|
|
|
|
|
+- [ ] **12.10 Fix string comparison in `highest-col` and `order-two-cols`**
|
|
|
+
|
|
|
+ Both functions call `(apply max ...)` on sequences of column-letter strings. ClojureScript's `max` is defined for numbers only — it does not perform lexicographic comparison on strings. This likely produces incorrect results for any column comparison and will break entirely once multi-letter columns (AA, AB, …) are introduced. Replace with an explicit comparator (e.g. `(apply max-key #(vector (count %) %) ...)` or a sort-based approach).
|
|
|
+
|
|
|
+- [ ] **12.11 Make `parse-range` fail loudly on malformed input**
|
|
|
+
|
|
|
+ `parse-range` runs four independent `re-find` calls on the same string. If any one fails to match, it silently passes `nil` into `range->list`, which cascades into bad data with no error surfaced to the user. Either validate the full string with a single comprehensive regex up front, or add an assertion/guard that all four captures succeeded.
|
|
|
+
|
|
|
### Deprecations
|
|
|
|
|
|
- [ ] **12.4 Replace deprecated `on-keyPress` with `on-key-down`**
|
|
|
|
|
|
The cell's enter-key handler in `views/sheet.cljs` uses `:on-keyPress`, which is deprecated in both React and the underlying browser API. Move to `:on-key-down`.
|
|
|
|
|
|
+- [ ] **12.12 Replace deprecated `reagent/render` with `reagent.dom/render`**
|
|
|
+
|
|
|
+ `core.cljs` calls `reagent/render`, which was deprecated in Reagent 1.x in favour of `reagent.dom/render`. Verify the installed Reagent version and update accordingly.
|
|
|
+
|
|
|
### Performance
|
|
|
|
|
|
- [ ] **12.5 Address unbounded `evaluate-expression` memoization cache**
|
|
|
@@ -273,3 +284,11 @@ When a cell or range is selected, show a compact set of floating action buttons
|
|
|
- [ ] **12.9 Rewrite `create-all-references` to use `walk-modify-data`**
|
|
|
|
|
|
Both functions perform a `reduce-kv` double-walk over the data map. Eliminate the duplication by having `create-all-references` call `walk-modify-data`.
|
|
|
+
|
|
|
+- [ ] **12.13 Convert `main-panel` to a form-2 Reagent component**
|
|
|
+
|
|
|
+ `main-panel` in `views.cljs` is a form-1 component that calls `re-frame/subscribe` at the top level. Re-frame's subscription cache makes this work in practice, but the canonical pattern for components holding subscriptions is form-2 — subscriptions are set up in the outer `defn`, and only the inner `fn` renders. Form-1 subscription calls can fail to clean up correctly across hot reloads.
|
|
|
+
|
|
|
+- [ ] **12.14 Call `dev-setup` before `mount-root` in `core.cljs`**
|
|
|
+
|
|
|
+ `init` currently calls `mount-root` before `dev-setup`. If `dev-setup` ever does real setup work (beyond a `println`), it will run after the first render. Swap the order so the dev environment is fully configured before any rendering occurs.
|
Brandon Wong