Repository navigation
Fix element cache edge cases - #257
Merged
Merged
Conversation
- decide cacheability once at mount, like the hash itself, so an instance that gets a uniqueHash later doesn't fill the cache with entries keyed on its random hash that nothing can reuse - generate the hash only when uniquifyIDs is on, since it's unused otherwise - document that the element cache is shared and independent of CacheProvider
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the element-cache eligibility fix and accompanying tooling updates.
No open issues 🎉
What changed in this PR
Fixes element-cache eligibility in react-inlinesvg when uniqueHash changes after mount, while updating development tooling and CI.
Changes:
- Determines hash stability at mount and generates hashes only when
uniquifyIDsis enabled. - Adds regression coverage and clarifies the element cache’s shared scope and independence from
CacheProvider. - Upgrades dependencies, restores Vitest matcher types, pins GitHub Actions, and adds formatting to validation.
| File | Description |
|---|---|
test/tsconfig.json |
Includes Node and jest-dom types. |
test/cacheElements.spec.tsx |
Tests adding uniqueHash after mount. |
test/__setup__/vitest.setup.ts |
Restores matcher types for Vitest 5. |
src/types.ts |
Documents mount-time hash requirements for caching. |
src/modules/useInlineSVG.ts |
Fixes hash-stability tracking and avoids unused hash generation. |
sonar-project.properties |
Corrects the Sonar source-directory property. |
README.md |
Clarifies cache behavior and formats documentation. |
pnpm-workspace.yaml |
Raises the Vite override and adds a release-age exclusion. |
package.json |
Upgrades tooling and updates validation scripts. |
CLAUDE.md |
Updates tooling and element-cache guidance. |
.github/workflows/ci.yml |
Updates actions and pins them to commit SHAs. |
.github/ISSUE_TEMPLATE/bug_report.yml |
Removes trailing whitespace. |
Review effort: Balanced
- bump vitest to 5, eslint to 10, jsdom to 30, jest-dom to 7 - restore jest-dom and jest-extended matcher types for vitest 5's Assertion<R, T> - raise the vite override to 8.3.5 to clear GHSA advisories in <=8.1.5 - pin GitHub Actions to commit SHAs
|
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.
uniquifyIDsthat got auniqueHashafter mounting wrote entries keyed on its random hash, which nothing could reuse and which pushed useful entries out.uniquifyIDsis on. It's unused otherwise.CacheProvider.Upgrade dependencies and actions
Assertion<R, T>, until Vitest 5: matcher types are lost —Assertion<T>augmentation no longer merges with Vitest'sAssertion<R, T>testing-library/jest-dom#738 ships.formattovalidate.