search

atom / command-palette

Command Palette in Atom
MIT License
116 stars 56 forks source link

Use cached elements when reactivating palette [fix] #95

Closed jarle closed 6 years ago

jarle commented 6 years ago

This PR fixes a problem from #94 where the item element cache was not utilized properly between palette toggles.

nathansobo commented 6 years ago

Is there a natural time to remove items from the cache? I'm not sure it will really matter because we're talking about a set of hundreds of small objects whose contents don't change much over the lifetime of an Atom window. But it's worth considering. @leroix I think I'm cool with this but I'd love to get your sign-off as well.

jarle commented 6 years ago

Oddly enough, the caching using WeakMap works just fine in 1.21.0-beta0, but not in 1.20.0.

Comparison toggling command palette 5-6 times(no manual GC): 1.21.0-beta0: screenshot_20171117_175923

1.20.0: screenshot_20171117_175238

jarle commented 6 years ago

Turns out I was running an outdated version of atom, so #94 should work as intended without this PR :smile: I will add a test case for keeping the cache between launches in a separate PR, just in case.