Tech - Nettoyage tech. #5176
Open
louptheron wants to merge 1 commit into
Open
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
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.



Linked issues
during development, Vite's "Fast Refresh" swaps in your new code while keeping
the app's state (open panels, form contents, etc.). It can only do that safely
if the file exports only components. Mix in a utility function and every edit to
that file throws away your state and reloads. Severity: zero impact on your
users — this purely makes development slower and more annoying. The fun finding:
most of the flagged files weren't component files at all — they were plain
TypeScript wearing a .tsx costume, which is also what confused the tool.
zero JSX)
.ts (styled-components only, no JSX; all imports are extensionless so nothing
else changed)
src/utils/getEnvironmentBorderStyle.ts (matching the one-function-per-file
convention there); updated its 3 importers
initialOffsetValue and computeTriangleMargins; I checked, nothing imports them
Pure function rebuilt every render (×13). Each of these helpers gets
recreated from scratch every single time the component redraws — even though it
never uses anything from inside the component. It's like rewriting the same
sticky note every time you glance at it. Severity: minor. Each allocation is
cheap; the real cost is that a "new" function each render can defeat memoized
children (making React redo work it could have skipped). Users wouldn't notice
on these screens, it's mostly hygiene. I verified each helper against the rule's
false-positive checklist (none touch props, state, or other local values) and
moved all 13 to module scope: onConnect (Login), handleRedirect (RequireAuth),
findStage (BeaconMalfunctionBoard), openXML (LogbookMessage),
getPercentOfTotalWeight (PNOMessageResume — which even had a TODO saying to do
exactly this), 2 chart helpers (SpeciesAndWeightChart), filterDuplicateSpecies
(LANMessageResume), handleCenterOnMap ×2 (both FormikCoordinatesPickers),
handleZoomToMission (MissionList), getInitialTitleVisibility (ReportingForm),
getText (InfractionsSummary).
Await inside a loop (×6, 3 real). await inside a loop means each operation
waits for the previous one to fully finish — like a cashier who won't start
scanning your items until the previous customer has driven out of the parking
lot. Severity: no production impact — all 3 real cases are in the puppeteer e2e
harness. It launches 2 browsers, connects to them, and closes them
one-at-a-time; now they run concurrently, shaving a few seconds off every
multi-windows test run. Order is preserved (Promise.all keeps array order,
window positions still come from the index).