🔥 Remove 'change' events

See #130

CHANGELOG.md

@@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Removed
 - `Movie#autoRefresh` (see [#130](https://github.com/etro-js/etro/issues/130)).
+- `change` events (see [#130](https://github.com/etro-js/etro/issues/130)).
+- `watchPublic()` and `publicExcludes` (see [#130](https://github.com/etro-js/etro/issues/130)).
 
 ### Fixed
 - `Movie#play()` and `Movie#record()` now wait until all resources are loaded before starting.

spec/integration/layer.spec.ts

@@ -2,49 +2,6 @@ import etro from '../../src/index'
 
 describe('Integration Tests ->', function () {
   describe('Layers', function () {
-    describe('Base', function () {
-      let layer
-
-      beforeEach(function () {
-        layer = new etro.layer.Base({ startTime: 0, duration: 4 })
-      })
-
-      it('should propagate changes up', function () {
-        // Connect to movie to publish event to
-        const movie = new etro.Movie({
-          canvas: document.createElement('canvas')
-        })
-        layer.tryAttach(movie)
-
-        // Listen for event called on movie
-        let timesFired = 0
-        etro.event.subscribe(movie, 'movie.change.layer', () => {
-          timesFired++
-        })
-        // Modify layer
-        layer.startTime = 1
-        expect(timesFired).toBe(1)
-      })
-
-      it('should not fire a change event when its active state changes', function () {
-        // Connect to movie to publish event to
-        const movie = new etro.Movie({
-          canvas: document.createElement('canvas')
-        })
-        layer.tryAttach(movie)
-
-        // Listen for event called on movie
-        let timesFired = 0
-        etro.event.subscribe(layer, 'layer.change', () => {
-          timesFired++
-        })
-
-        // Update active state
-        layer.active = true
-        expect(timesFired).toBe(0)
-      })
-    })
-
     describe('Visual', function () {
       class CustomEffect extends etro.effect.Base {
         private _ready = false

spec/integration/movie.spec.ts

@@ -335,15 +335,6 @@ describe('Integration Tests ->', function () {
         expect(firedOnce).toBe(true)
       })
 
-      it("should fire 'movie.change.modify'", function () {
-        let timesFired = 0
-        etro.event.subscribe(movie, 'movie.change.modify', function () {
-          timesFired++
-        })
-        movie.currentTime = movie.duration / 2
-        expect(timesFired).toBe(1)
-      })
-
       it("should publish 'movie.ready' when its layers and effects become ready", function (done) {
         // Remove all layers and effects
         movie.layers.length = 0

spec/unit/mocks/effect.ts

@@ -1,8 +1,6 @@
-import etro from '../../../src'
-
 // eslint-disable-next-line no-unused-vars
-export function mockBaseEffect (watchPublic = false) {
-  let effect = jasmine.createSpyObj('effect', [
+export function mockBaseEffect () {
+  const effect = jasmine.createSpyObj('effect', [
     'getDefaultOptions',
     'tryAttach',
     'tryDetach'
@@ -14,10 +12,6 @@ export function mockBaseEffect (watchPublic = false) {
     effect.movie = movie
   })
 
-  // I believe `watchPublic` needs to be called before we add the properties.
-  if (watchPublic)
-    effect = etro.watchPublic(effect) as etro.effect.Base
-
   effect.type = 'effect'
   effect.enabled = true
   effect.ready = true

spec/unit/mocks/layer.ts

@@ -1,9 +1,8 @@
-import etro from '../../../src'
 import { mockMovie } from './movie'
 
 // eslint-disable-next-line no-unused-vars
-export function mockBaseLayer (watchPublic = false) {
-  let layer = jasmine.createSpyObj('layer', [
+export function mockBaseLayer () {
+  const layer = jasmine.createSpyObj('layer', [
     'getDefaultOptions',
     'tryAttach',
     'tryDetach',
@@ -18,10 +17,6 @@ export function mockBaseLayer (watchPublic = false) {
     layer.movie = movie
   })
 
-  // I believe `watchPublic` needs to be called before we add the properties.
-  if (watchPublic)
-    layer = etro.watchPublic(layer) as etro.layer.Base
-
   layer.type = 'layer'
   layer.active = false
   layer.enabled = true

spec/unit/mocks/movie.ts

@@ -1,9 +1,8 @@
-import etro from '../../../src'
 import { mockAudioContext, mockCanvas } from './dom'
 
 // eslint-disable-next-line no-unused-vars
-export function mockMovie (watchPublic = false) {
-  let movie = jasmine.createSpyObj('movie', [
+export function mockMovie () {
+  const movie = jasmine.createSpyObj('movie', [
     'getDefaultOptions',
     'addLayer',
     'addEffect'
@@ -12,10 +11,6 @@ export function mockMovie (watchPublic = false) {
     canvas: null
   })
 
-  // I believe `watchPublic` needs to be called before we add the properties.
-  if (watchPublic)
-    movie = etro.watchPublic(movie) as etro.Movie
-
   movie.enabled = true
   movie.type = 'movie'
   movie.publicExcludes = []

spec/unit/util.spec.ts

@@ -200,105 +200,5 @@ describe('Unit Tests ->', function () {
           .toEqual(new etro.Font(16, 'px', '"Times New Roman"'))
       })
     })
-
-    describe('watchPublic', function () {
-      it('should watch existing public properties', function () {
-        const layer = mockBaseLayer(true)
-        const history = []
-        etro.event.subscribe(layer, 'layer.change.modify', event => history.push(event))
-        layer.enabled = false
-
-        expect(history).toEqual([
-          {
-            target: layer,
-            type: 'layer.change.modify',
-            property: 'enabled',
-            newValue: false
-          }
-        ])
-      })
-
-      it('should not watch existing public properties in `publicExcludes`', function () {
-        // Create a fake etro element and watch it
-        const layer = mockBaseLayer(true)
-        layer.publicExcludes = ['enabled']
-        // Initialize (must be after watchPublic)
-        layer.enabled = false
-        // Record matching events
-        const history = []
-        etro.event.subscribe(layer, 'layer.change.modify', event => history.push(event))
-
-        // Modify property
-        layer.enabled = true
-
-        // It should have emitted one event
-        expect(history).toEqual([])
-      })
-
-      it('should watch for modifications on existing public property of child object', function () {
-        const movie = mockMovie(true)
-        const history = []
-        etro.event.subscribe(movie, 'movie.change.modify', event => history.push(event))
-
-        movie.canvas.width = 100
-        expect(history).toEqual([
-          {
-            target: movie,
-            type: 'movie.change.modify',
-            property: 'canvas.width',
-            newValue: 100
-          }
-        ])
-      })
-
-      it("should respect a child etro element's `publicExcludes`", function () {
-        // Consider a Etro element `child`, which is a child of etro element
-        // `parent`. The parent should not watch properties on the child that are
-        // in `child.publicExcludes`.
-
-        class Child implements etro.EtroObject {
-          propertyFilters: Record<string, <T>(value: T) => T>
-          type = 'child'
-          publicExcludes = ['bar']
-          movie: etro.Movie
-          ready = true
-          currentTime = 0
-
-          bar: number
-
-          getDefaultOptions (): object {
-            return {}
-          }
-        }
-
-        class Parent implements etro.EtroObject {
-          propertyFilters: Record<string, <T>(value: T) => T>
-          type = 'parent'
-          publicExcludes = ['bar']
-          movie: etro.Movie
-          ready = true
-          currentTime = 0
-
-          child: Child
-
-          getDefaultOptions (): object {
-            return {}
-          }
-        }
-
-        // Setup
-        const parent = new Parent()
-        const child = new Child()
-        child.bar = 0
-        parent.child = child
-        const history = []
-        etro.event.subscribe(parent, 'movie.change.modify', event => history.push(event))
-
-        // Modify child.foo
-        child.bar = 100
-
-        expect(history).toEqual([])
-      })
-    })
   })
 })

src/effect/base.ts

@@ -1,5 +1,3 @@
-import { watchPublic } from '../util'
-import { publish, subscribe } from '../event'
 import { Movie } from '../movie'
 import { Base as BaseLayer } from '../layer/index'
 import BaseObject from '../object'
@@ -23,22 +21,9 @@ export class Base implements BaseObject {
   private _occurrenceCount: number
 
   constructor () {
-    const newThis = watchPublic(this) as Base // proxy that will be returned by constructor
-
-    newThis.enabled = true
-    newThis._occurrenceCount = 0
-    newThis._target = null
-
-    // Propagate up to target
-    subscribe(newThis, 'effect.change.modify', event => {
-      if (!newThis._target)
-        return
-
-      const type = `${newThis._target.type}.change.effect.modify`
-      publish(newThis._target, type, { ...event, target: newThis._target, source: newThis, type })
-    })
-
-    return newThis
+    this.enabled = true
+    this._occurrenceCount = 0
+    this._target = null
   }
 
   /**

src/effect/stack.ts

@@ -51,7 +51,6 @@ export class Stack extends Visual {
 
     this.effects = new StackEffects(options.effects, this)
     options.effects.forEach(effect => this.effects.push(effect))
-    // TODO: Propagate 'change' events from children up
   }
 
   attach (movie: Movie): void {

src/layer/base.ts

@@ -1,6 +1,5 @@
 import EtroObject from '../object'
-import { publish, subscribe } from '../event'
-import { watchPublic, applyOptions } from '../util'
+import { applyOptions } from '../util'
 import { Movie } from '../movie'
 
 interface BaseOptions {
@@ -53,10 +52,6 @@ class Base implements EtroObject {
     this._startTime = options.startTime
     this._duration = options.duration
 
-    // Proxy that will be returned by constructor (for sending 'modified'
-    // events).
-    const newThis = watchPublic(this) as Base
-    // Don't send updates when initializing, so use this instead of newThis
     applyOptions(options, this)
 
     // Whether this layer is currently being rendered
@@ -65,15 +60,6 @@ class Base implements EtroObject {
 
     this._occurrenceCount = 0 // no occurrences in parent
     this._movie = null
-
-    // Propagate up to target
-    subscribe(newThis, 'layer.change', event => {
-      const typeOfChange = event.type.substring(event.type.lastIndexOf('.') + 1)
-      const type = `movie.change.layer.${typeOfChange}`
-      publish(newThis._movie, type, { ...event, target: newThis._movie, type })
-    })
-
-    return newThis
   }
 
   /**

src/movie/effects.ts

@@ -1,6 +1,6 @@
 import { CustomArray, CustomArrayListener } from '../custom-array'
 import { Visual as VisualEffect } from '../effect/index'
-import { publish, subscribe } from '../event'
+import { subscribe } from '../event'
 import { Movie } from './movie'
 
 class MovieEffectsListener extends CustomArrayListener<VisualEffect> {
@@ -15,9 +15,6 @@ class MovieEffectsListener extends CustomArrayListener<VisualEffect> {
 
   onAdd (effect: VisualEffect) {
     effect.tryAttach(this._movie)
-    // Refresh screen when effect is set, if the movie isn't playing
-    // already.
-    publish(this._movie, 'movie.change.effect.add', { effect })
 
     // Update ready state if the effect is not ready
     this._checkReady()
@@ -34,9 +31,6 @@ class MovieEffectsListener extends CustomArrayListener<VisualEffect> {
 
   onRemove (effect: VisualEffect) {
     effect.tryDetach()
-    // Refresh screen when effect is removed, if the movie isn't playing
-    // already.
-    publish(this._movie, 'movie.change.effect.remove', { effect })
 
     // Update ready state if the effect was not ready
     this._checkReady()

src/movie/layers.ts

@@ -1,6 +1,6 @@
 import { CustomArray, CustomArrayListener } from '../custom-array'
 import { Base as BaseLayer } from '../layer/index'
-import { publish, subscribe } from '../event'
+import { subscribe } from '../event'
 import { Movie } from './movie'
 
 class MovieLayersListener extends CustomArrayListener<BaseLayer> {
@@ -16,15 +16,6 @@ class MovieLayersListener extends CustomArrayListener<BaseLayer> {
   onAdd (layer: BaseLayer) {
     layer.tryAttach(this._movie)
 
-    if (
-      this._movie.currentTime >= layer.startTime &&
-      this._movie.currentTime < layer.startTime + layer.duration
-    )
-      publish(this._movie, 'movie.change.layer.add', { layer })
-
-    const oldDuration = Math.max(this._movie.duration, layer.startTime + layer.duration)
-    publish(this._movie, 'movie.change.duration', { oldDuration })
-
     // Update internal ready state if the layer is not ready
     this._checkReady()
 
@@ -40,15 +31,6 @@ class MovieLayersListener extends CustomArrayListener<BaseLayer> {
   onRemove (layer: BaseLayer) {
     layer.tryDetach()
 
-    if (
-      this._movie.currentTime >= layer.startTime &&
-      this._movie.currentTime < layer.startTime + layer.duration
-    )
-      publish(this._movie, 'movie.change.layer.remove', { layer })
-
-    const oldDuration = Math.max(this._movie.duration, layer.startTime + layer.duration)
-    publish(this._movie, 'movie.change.duration', { oldDuration })
-
     // Update internal ready state if the layer was not ready
     this._checkReady()
   }