sindresorhus / emoj

Find relevant emoji from text on the command-line :open_mouth: :sparkles: :raised_hands: :horse: :boom: :see_no_evil:
MIT License
2.37k stars 58 forks source link

Prevent setState after unmount #47

Closed spenserblack closed 4 years ago

spenserblack commented 4 years ago

Hello 👋

When running emoj with no arguments, then typing any string of characters, followed by pressing esc, I noticed that I was getting

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method.
    in Emoj (created by Context.Consumer)
    in Unknown
    in App

I narrowed this down to fetchEmojis attempting to set the state after the component had unmounted.

This PR prevents that warning by blocking fetchEmojis from attempting to set state after a user has input ESC.

I'm unfamiliar with using React components in CLI applications, so I apologize if these changes do not follow best practices.

sindresorhus commented 4 years ago

There must be a better way to resolve this than adding more state.

sindresorhus commented 4 years ago

// @vadimdemedes

spenserblack commented 4 years ago

There must be a better way to resolve this than adding more state.

Most likely 😆 . I originally put it in there in case you wanted to do something with an exiting state, but probably best to be left out after all. I just pushed up a different commit which doesn't set state. Instead it just uses this.exiting. I opted not to store the value in props, since it's a value that should always be false on instantiation.

But I feel like there's still a better solution somewhere 🤔 . Setting an exiting variable still feels a bit inelegant 😆 .

vadimdemedes commented 4 years ago

I think this should no longer be an issue after https://github.com/sindresorhus/emoj/pull/46 is merged.

sindresorhus commented 4 years ago

Fixed by https://github.com/sindresorhus/emoj/pull/46