Skip to content

Conversation

@ngokevin
Copy link
Member

@ngokevin ngokevin commented Dec 18, 2017

Description:

Emit was potentially creating object in three spots:

  • data = {bubbles: !!bubbles, detail: detail}
  • data = data || {};
  • data.detail = data.detail || {};

This PR makes it to reuse the same object on each emit, except if extraData is provided, in which that customizes the event and we need a separate object.

There is no worry of cross-contamination. The CustomEvent data parameter will pass the object properties along to the event object, but the data parameter objects themselves aren't exposed.

I removed the indirection to the utils.fireEvent (unused, undocumented). It seems its purpose was to create objects if necessary which is no longer needed. It also added detail.target which is not needed because the browser natively has event.target and event.currentTarget. Also made it confusing when event detail objects are reused at application level, the event.detail.target was often incorrect in my experience through contamination.

Changes proposed:

  • Get rid of object allocations in .emit() except in the case of extraData.
  • Don't allocate empty object for nodeready event, just pass undefined.

@dmarcos dmarcos merged commit 19643e7 into aframevr:master Dec 18, 2017
ngokevin added a commit to ngokevin/aframe that referenced this pull request Jan 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants