Fix late initialization error in DevTools store logic#9816
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reorders the initialization logic in the DevToolsUsage constructor, moving the _removeLegacyKeys call after the properties object is initialized to resolve a late initialization error. While this change fixes the immediate initialization issue, feedback was provided regarding the persistence of the cleanup logic; currently, _removeLegacyKeys only removes keys from memory, meaning they will reappear on subsequent runs unless the removal is properly persisted to disk.
| storeName, | ||
| documentDirPath: LocalFileSystem.devToolsDir(), | ||
| ); | ||
| _removeLegacyKeys(); |
There was a problem hiding this comment.
[CONCERN] While moving this call fixes the late initialization error, the _removeLegacyKeys method (and the IOPersistentProperties.remove method it calls) does not currently persist changes to disk. This means legacy keys are only removed from memory and will reappear on the next run. Consider updating _removeLegacyKeys to use properties[key] = null or fixing IOPersistentProperties.remove to ensure the cleanup is permanent.
References
- The style guide emphasizes focusing on logic and architectural consistency. The current implementation of the cleanup logic is logically flawed as it lacks persistence. (link)
2 participants
Required for 2.58.0 release.