Skip to content

Conversation

@MartinSchoeler
Copy link
Member

@MartinSchoeler MartinSchoeler commented Jul 15, 2020

@MartinSchoeler MartinSchoeler marked this pull request as ready for review July 16, 2020 19:19
@MartinSchoeler MartinSchoeler requested a review from a team July 16, 2020 19:20
@MartinSchoeler MartinSchoeler changed the title [NEW] Enterprise tag [NEW] Enterprise tags Jul 16, 2020
@tassoevan tassoevan changed the base branch from develop to enterprise-license-tags July 16, 2020 20:25
@tassoevan tassoevan added area: ui Touches the code on client side type: improvement labels Jul 16, 2020
@tassoevan tassoevan added this to the 3.5.0 milestone Jul 16, 2020
@tassoevan tassoevan changed the base branch from enterprise-license-tags to develop July 16, 2020 20:28
@tassoevan tassoevan changed the base branch from develop to enterprise-license-tags July 16, 2020 20:28
This reverts commit 8abf038, reversing
changes made to 99d6589.

useMemo(() => {
const loadTags = async () => {
setPlans(await getTags());
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a catch here: setPlans can be accidentally called after the component was unmounted, i.e., PlanTag may be already unmounted after the getTags() returned promise was resolved. To avoid that we usually do some kind of semaphore to skip the setPlans calls after unmount, but I've made a custom hook for that:

import { useSafely } from '@rocket.chat/fuselage-hooks';

const [plans, setPlans] = useSafely(useState([]));

So setPlans can be called anytime.

N.B.: Pay attention to possible memory leaks, tough. If the promise never resolves, setPlans will be referenced forever, and the whole component as well.

MartinSchoeler and others added 2 commits July 16, 2020 19:15
@lgtm-com
Copy link

lgtm-com bot commented Jul 17, 2020

This pull request introduces 3 alerts when merging fb443fe into f2ef91d - view on LGTM.com

new alerts:

  • 2 for Assignment to constant
  • 1 for Unused variable, import, function or class

@ggazzo ggazzo changed the base branch from enterprise-license-tags to develop July 17, 2020 03:39
@ggazzo ggazzo changed the base branch from develop to enterprise-license-tags July 17, 2020 03:39
@ggazzo ggazzo merged commit 96c21d8 into enterprise-license-tags Jul 17, 2020
@ggazzo ggazzo deleted the enterprise-tag branch July 17, 2020 03:45
@sampaiodiego sampaiodiego restored the enterprise-tag branch July 17, 2020 14:58
sampaiodiego added a commit that referenced this pull request Jul 17, 2020
sampaiodiego added a commit that referenced this pull request Jul 17, 2020
From #18276

Co-authored-by: Tasso Evangelista <[email protected]>
Co-authored-by: Guilherme Gazzo <[email protected]>
Co-authored-by: Martin Schoeler <[email protected]>
@sampaiodiego sampaiodiego deleted the enterprise-tag branch July 17, 2020 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ui Touches the code on client side type: improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants