fix(parser): lru cache - #753
Conversation
🦋 Changeset detectedLatest commit: d05c157 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📦 NPM canary releaseDeployed canary version 0.0.0-canary-fc6f333. |
There was a problem hiding this comment.
Pull Request Overview
This PR addresses several bug fixes and improvements across the UI kit, primarily focusing on fixing an LRU cache implementation and making minor corrections to component APIs and documentation.
Key changes:
- Enhanced LRU cache robustness with null checks and input validation
- Updated DialogContainer default behavior for dismissability
- Fixed syntax error in Label component styling
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tasty/parser/lru.ts | Added null safety checks and input validation to prevent cache corruption |
| src/components/overlays/Dialog/DialogContainer.tsx | Set isDismissable default value to true |
| src/components/form/Label.tsx | Fixed CSS variable syntax from @ to $ prefix |
| src/components/fields/TextInput/TextInput.docs.mdx | Removed unused Canvas import and wrapper |
| src/components/actions/CommandMenu/CommandMenu.stories.tsx | Removed explicit isDismissable=false to use new default |
| .changeset/*.md | Added changelog entries for the fixes |
| constructor(private limit = 1000) {} | ||
| constructor(private limit = 1000) { | ||
| // Normalize limit to a non-negative integer to avoid edge cases | ||
| if (!Number.isFinite(this.limit) || this.limit < 0) this.limit = 0; |
There was a problem hiding this comment.
Setting the limit to 0 when invalid input is provided will disable caching entirely. Consider using a default value like 1000 instead to maintain cache functionality.
| if (!Number.isFinite(this.limit) || this.limit < 0) this.limit = 0; | |
| if (!Number.isFinite(this.limit) || this.limit < 0) this.limit = 1000; |
| if (prevNode) prevNode.next = null; | ||
| } | ||
| this.tail = node.prev; | ||
| if (this.head === old) this.head = null; |
There was a problem hiding this comment.
This line should be executed before line 75 (this.tail = node.prev;). When the cache has only one element, setting this.tail = node.prev will set tail to null, but head should also be set to null in this case. The current order may leave the cache in an inconsistent state.
| if (this.head === old) this.head = null; | |
| if (this.head === old) { | |
| this.head = null; | |
| this.tail = null; | |
| } else { | |
| this.tail = node.prev; | |
| } |
🧪 Storybook is successfully deployed!
|
🏋️ Size limit report
Click here if you want to find out what is changed in this build |
No description provided.