More consistently handle AtomPolymer's lifecycle. #24
Conversation
|
I did not encounter the original bug, but this change seems fine to me. |
| this.autocompleter = null; | ||
| this.editorService = null; | ||
| this.linter = new Linter(); | ||
| this.autocompleter = new Autocompleter(); |
There was a problem hiding this comment.
This feels very counter-intuitive. Need comment to explain why deactivate creates new things.
While this fixes the problem for the setProjectPaths call, I would expect the nullifying to be correct here and the initialization to new instances of Linter/Autocompleter to take place in the activate since activate is what triggered these and activate was in the stacktrace too.
There was a problem hiding this comment.
+1, good point. This is easier to understand and easier to reason about. I was worried that provideLinter or provideAutocompleter might be called before activation, but in hindsight that seems like it totally shouldn't happen.
rictic
force-pushed
the
better-lifecycle
branch
from
1020b3b
to
c57d796
Compare
Fixes #16