Skip to content
This repository was archived by the owner on Aug 7, 2021. It is now read-only.

fix: don't throw MaxListenersExceeded error when more than 11 processes are started from sidekick #695

Merged
merged 1 commit into from
Oct 19, 2018

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Oct 18, 2018

In case when the user runs 11 apps from sidekick (or only 2 apps changing them 6 times),

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 exit listeners added. Use emitter.setMaxListeners() to increase limit

error is thrown. The problem occurs because we attach every time on process.exit and liveSyncStopped events. So this PR fixes this behavior as ensures only one listener is added for each event.

PR Checklist

What is the current behavior?

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 exit listeners added. Use emitter.setMaxListeners() to increase limit

error is thrown when the user runs 11 apps from sidekick (or only 2 apps changing them 6 times)

What is the new behavior?

No error is thrown when the user runs 11 apps from sidekick (or only 2 apps changing them 6 times)

Copy link
Contributor

@sis0k0 sis0k0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are ok. Just return the spaces instead of tabs.

Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the current implementation you'll still receive the same warning in case you open 11 different projects in Sidekick (in a single instance) and run them. I think more appropriate solution is:

  1. Attach only once on process.exit and liveSyncStopped event, i.e. keep the boolean flag here.
  2. Whenever livesyncStopped is raised, check if you still have processes and if not - remove the process.exit and liveSyncStopped event handlers. You can do it here and reset the variable or do it in the after-watch hook.

Basically we would not need the boolean variable in case we follow the approach below:

  1. On before-watch attach liveSyncStopped and process.exit handlers
  2. On after-watch remove the handlers

This way, when you close a project in Sidekick, after-watch hook will be executed and we'll remove the handlers we've attached. Starting liveSync on another project will add new handlers, but we'll never add more than one process.exit handler for example (as you cannot have multiple projects working in Sidekick)

@Fatme Fatme force-pushed the fatme/fix-max-listeners branch from bdf7ffb to 9782e44 Compare October 18, 2018 10:07
@Fatme
Copy link
Contributor Author

Fatme commented Oct 18, 2018

run ci

@Fatme Fatme merged commit 79490c7 into master Oct 19, 2018
@Fatme Fatme deleted the fatme/fix-max-listeners branch October 19, 2018 03:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants