Merge pull request #3667 from atom/ns-deserializer-manager-disposables

Return disposables from DeserializerManager::add

spec/deserializer-manager-spec.coffee

@@ -1,33 +1,44 @@
 DeserializerManager = require '../src/deserializer-manager'
 
-describe ".deserialize(state)", ->
-  deserializer = null
+describe "DeserializerManager", ->
+  manager = null
 
   class Foo
     @deserialize: ({name}) -> new Foo(name)
     constructor: (@name) ->
 
   beforeEach ->
-    deserializer = new DeserializerManager()
-    deserializer.add(Foo)
+    manager = new DeserializerManager
 
-  it "calls deserialize on the deserializer for the given state object, or returns undefined if one can't be found", ->
+  describe "::add(deserializer)", ->
+    it "returns a disposable that can be used to remove the manager", ->
+      disposable = manager.add(Foo)
+      expect(manager.deserialize({deserializer: 'Foo', name: 'Bar'})).toBeDefined()
+      disposable.dispose()
       spyOn(console, 'warn')
-    object = deserializer.deserialize({ deserializer: 'Foo', name: 'Bar' })
-    expect(object.name).toBe 'Bar'
-    expect(deserializer.deserialize({ deserializer: 'Bogus' })).toBeUndefined()
+      expect(manager.deserialize({deserializer: 'Foo', name: 'Bar'})).toBeUndefined()
 
-  describe "when the deserializer has a version", ->
+  describe "::deserialize(state)", ->
+    beforeEach ->
+      manager.add(Foo)
+
+    it "calls deserialize on the manager for the given state object, or returns undefined if one can't be found", ->
+      spyOn(console, 'warn')
+      object = manager.deserialize({deserializer: 'Foo', name: 'Bar'})
+      expect(object.name).toBe 'Bar'
+      expect(manager.deserialize({deserializer: 'Bogus'})).toBeUndefined()
+
+    describe "when the manager has a version", ->
       beforeEach ->
         Foo.version = 2
 
       describe "when the deserialized state has a matching version", ->
         it "attempts to deserialize the state", ->
-        object = deserializer.deserialize({ deserializer: 'Foo', version: 2, name: 'Bar' })
+          object = manager.deserialize({deserializer: 'Foo', version: 2, name: 'Bar'})
           expect(object.name).toBe 'Bar'
 
       describe "when the deserialized state has a non-matching version", ->
         it "returns undefined", ->
-        expect(deserializer.deserialize({ deserializer: 'Foo', version: 3, name: 'Bar' })).toBeUndefined()
-        expect(deserializer.deserialize({ deserializer: 'Foo', version: 1, name: 'Bar' })).toBeUndefined()
-        expect(deserializer.deserialize({ deserializer: 'Foo', name: 'Bar' })).toBeUndefined()
+          expect(manager.deserialize({deserializer: 'Foo', version: 3, name: 'Bar'})).toBeUndefined()
+          expect(manager.deserialize({deserializer: 'Foo', version: 1, name: 'Bar'})).toBeUndefined()
+          expect(manager.deserialize({deserializer: 'Foo', name: 'Bar'})).toBeUndefined()

spec/pane-container-view-spec.coffee

@@ -5,11 +5,11 @@ PaneView = require '../src/pane-view'
 {$, View, $$} = require 'atom'
 
 describe "PaneContainerView", ->
-  [TestView, container, pane1, pane2, pane3] = []
+  [TestView, container, pane1, pane2, pane3, deserializerDisposable] = []
 
   beforeEach ->
     class TestView extends View
-      atom.deserializers.add(this)
+      deserializerDisposable = atom.deserializers.add(this)
       @deserialize: ({name}) -> new TestView(name)
       @content: -> @div tabindex: -1
       initialize: (@name) -> @text(@name)
@@ -25,7 +25,7 @@ describe "PaneContainerView", ->
     pane3 = pane2.splitDown(new TestView('3'))
 
   afterEach ->
-    atom.deserializers.remove(TestView)
+    deserializerDisposable.dispose()
 
   describe ".getActivePaneView()", ->
     it "returns the most-recently focused pane", ->

spec/pane-spec.coffee

@@ -4,6 +4,8 @@ PaneAxis = require '../src/pane-axis'
 PaneContainer = require '../src/pane-container'
 
 describe "Pane", ->
+  deserializerDisposable = null
+
   class Item extends Model
     @deserialize: ({name, uri}) -> new this(name, uri)
     constructor: (@name, @uri) ->
@@ -13,10 +15,10 @@ describe "Pane", ->
     isEqual: (other) -> @name is other?.name
 
   beforeEach ->
-    atom.deserializers.add(Item)
+    deserializerDisposable = atom.deserializers.add(Item)
 
   afterEach ->
-    atom.deserializers.remove(Item)
+    deserializerDisposable.dispose()
 
   describe "construction", ->
     it "sets the active item to the first item", ->

spec/pane-view-spec.coffee

@@ -7,7 +7,7 @@ path = require 'path'
 temp = require 'temp'
 
 describe "PaneView", ->
-  [container, containerModel, view1, view2, editor1, editor2, pane, paneModel] = []
+  [container, containerModel, view1, view2, editor1, editor2, pane, paneModel, deserializerDisposable] = []
 
   class TestView extends View
     @deserialize: ({id, text}) -> new TestView({id, text})
@@ -23,7 +23,7 @@ describe "PaneView", ->
       @emitter.on 'did-change-title', callback
 
   beforeEach ->
-    atom.deserializers.add(TestView)
+    deserializerDisposable = atom.deserializers.add(TestView)
     container = new PaneContainerView
     containerModel = container.model
     view1 = new TestView(id: 'view-1', text: 'View 1')
@@ -40,7 +40,7 @@ describe "PaneView", ->
       paneModel.addItems([view1, editor1, view2, editor2])
 
   afterEach ->
-    atom.deserializers.remove(TestView)
+    deserializerDisposable.dispose()
 
   describe "when the active pane item changes", ->
     it "hides all item views except the active one", ->

src/deserializer-manager.coffee

@@ -1,3 +1,6 @@
+{Disposable} = require 'event-kit'
+Grim = require 'grim'
+
 # Extended: Manages the deserializers used for serialized state
 #
 # An instance of this class is always available as the `atom.deserializers`
@@ -24,14 +27,17 @@ class DeserializerManager
 
   # Public: Register the given class(es) as deserializers.
   #
-  # * `classes` One or more classes to register.
-  add: (classes...) ->
-    @deserializers[klass.name] = klass for klass in classes
+  # * `deserializers` One or more deserializers to register. A deserializer can
+  #   be any object with a `.name` property and a `.deserialize()` method. A
+  #   common approach is to register a *constructor* as the deserializer for its
+  #   instances by adding a `.deserialize()` class method.
+  add: (deserializers...) ->
+    @deserializers[deserializer.name] = deserializer for deserializer in deserializers
+    new Disposable =>
+      delete @deserializers[deserializer.name] for deserializer in deserializers
 
-  # Public: Remove the given class(es) as deserializers.
-  #
-  # * `classes` One or more classes to remove.
   remove: (classes...) ->
+    Grim.deprecate("Call .dispose() on the Disposable return from ::add instead")
     delete @deserializers[name] for {name} in classes
 
   # Public: Deserialize the state and params.