Replace usages of Events with Emitter

Question

Replace usages of Events with Emitter

Closed this issue 4 years ago · 8 comments

Events.js is deprecated and Emitter should be used instead, we should chip away at this. This is somewhat related to #102 because PropertySet extended Events.

Answer 1 · 2017-09-10T14:08:40.000Z

Scheduled for developer meeting to do the following:

Make sure this is a desirable task to complete
Determine what to do with usages in Node.js. Should this file become Node-specific, or integrated into Node? Should Node use emitters? How many emitters would each Node need to evade Events?
Schedule/plan for working on this

Answer 2 · 2017-09-12T18:08:20.000Z

Searching for "var Events = require", I see the following number of usages by repository:

common code:
1 axon (PropertySet)
2 dot (Transform3, Transform4)
3 kite (Shape, Segments, Subpath)
6 scenery (Node, Display, Drawable, Instance, AccessibleInstance, AccessiblePeer)

sims:
3 fluid-pressure-and-flow
1 molecule-shapes

Thoughts:

(1) Replace usages of "var Events = require" with "var Emitter = require" in sim repos first. This should be relative easy.

(2) Node extends Events has always felt a bit wrong. I don't think that Node should extend Emitter. But changing from inheritance to (e.g.) composition will complicate changes to client code.

(3) Use of the Events API for Node subtypes will be probably be the bulk of the changes to client code. This will be time-consuming and (potentially) destabilizing to locate and change.

[EDITED by @pixelzoom]

Answer 3 · 2017-09-12T18:16:28.000Z

1 axon (Property)

Property and Node both extend Events.

Did you mean PropertySet? That currently extends Events and will be deleted once it is no longer used.

Answer 4 · 2017-09-12T18:29:34.000Z

Yes, I meant PropertySet. Looks like you didn't see the EDIT to my comment above.

Answer 5 · 2017-09-14T18:56:23.000Z

9/14/17 dev meeting:

Start by removing var Events from sims.
Then remove from dot and kite.
Then address scenery (where some of the usage is internal).

All of the above except FAPF is in @jonathanolson's wheelhouse, so he self assigned.

Answer 6 · 2017-09-15T15:10:53.000Z

Co-assigning to @zepumph to work on the FPAF occurrences. It looks like all of the occurrences trigger update or updated, so these should all change to updatedEmitter.emit.

Answer 7 · 2017-09-18T19:41:44.000Z

FPAF was completed in phetsims/fluid-pressure-and-flow#297. Unassigning.

Answer 8 · 2021-02-19T17:25:03.000Z

Events has been deleted, closing.