Chore/cleanup and comment rationale#35
Merged
Merged
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the ImageObject component to handle image cache resets during the render phase rather than within an effect, which aligns with React best practices for prop-driven state updates. Additionally, it removes redundant TypeScript type assertions, adds explanatory comments for complex casts in BarcodeObject, and deletes a significant amount of unused CSS. I have no feedback to provide as there were no review comments to evaluate.
Leftover from the Vite template — defines .counter, .hero, #docs etc. which the app never references. Tailwind is loaded via index.css and the @tailwindcss/vite plugin.
After narrowing obj.type to 'text' or 'serial', TypeScript already narrows obj.props to TextProps | SerialProps via the LabelObject discriminated union. Both members declare fontHeight: number and rotation: 'N'|'R'|'I'|'B', so the inline cast was just widening rotation to string for no reason.
ImageObject reset its htmlImg state from inside useEffect with an eslint-disable for react-hooks/set-state-in-effect. Switch to the official React 'adjusting state on prop change during render' pattern: a useRef tracks the previous cached.dataUrl, and a render-time comparison resets htmlImg before paint. Same observable behaviour, no lint exception, and the next render naturally observes the updated ref so there's no loop.
The cast bridges our Record<string, unknown> options builder (which varies per barcode type) and bwip-js' strict literal-string union signature. Document the trade-off so future readers don't try to 'fix' the cast by re-typing buildBwipOptions per type — that would duplicate the per-type switch already inside buildBwipOptions.
3da4bf7 to
4c18afc
Compare
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.
No description provided.