Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onClick does not see state hooks changes #748

Closed
crosskpixel opened this issue Jun 22, 2019 · 16 comments
Closed

onClick does not see state hooks changes #748

crosskpixel opened this issue Jun 22, 2019 · 16 comments

Comments

@crosskpixel
Copy link

I am using react 16.8 hooks, in the map component I define an onClick function, but when I retrieve the value of the state hooks me the initial value is returned.

in the visualization it is possible to see the updated value of count, but within the function it is not possible

const [count,setCount] = useState(0); ... .... ... onClick={(map, evt) => { console.log(count); // Caso o indicador criando marcador esteja ativo ira criar marcadores no mapa. const { lng, lat } = evt.lngLat; setCriandoMarcador(true); if (criandoMarcador) { setPontosGps([...pontosGps, { lng, lat }]); if (!travarFerramenta) { setCriandoMarcador(false); } } }}

@mklopets
Copy link
Collaborator

Could you explain what you mean by "the visualization"?

It's possible that the onClick handler currently isn't change-able after mount, indeed. Don't think it has anything to do with hooks.

@vinceprofeta
Copy link

@alex3165 Can we get this merged?

@qlerebours
Copy link

I also noticed that the behavior of onClick is captured and never changed.
I will rewrite @crosskpixel example to be more comprehensible:

const ReactHookComponent = () => {
    const [count, setCount] = useState(0);

    useEffect(() => {
        setCount(c => c+1); // start by increment count from 0 to 1
    }, []);

    const handleClick = () => {
        console.log('Map clicked with count', count); // will display 'Map clicked with count 0'. State is not up to date
    }

   return (
        <Map onClick={handleClick} />
    )
}

At the moment, a crappy way to work this around is to use a ref:

    const countForMap = useRef(0);

    useEffect(() => {
       countForMap.current = count;
   }, [count])

   const handleClick = () => {
        console.log('Map clicked with count', countForMap.current); // will display 'Map clicked with count 1
    }

@qlerebours
Copy link

I dived into the code to understand why, because this was not the first time I faced this issue, and I think I found the mistake, but I will need @alex3165 to confirm this is a mistake.

In the src/map-events.tsx file, the updateEvents function is used when the map receives props.
Here is the function:

export const updateEvents = (
  listeners: Listeners,
  nextProps: Partial<Events>,
  map: MapboxGl.Map
) => {
  const toListenOff = Object.keys(events).filter(
    eventKey => listeners[eventKey] && typeof nextProps[eventKey] !== 'function'
  );

  toListenOff.forEach(key => {
    map.off(events[key], listeners[key]);
    delete listeners[key];
  });

  const toListenOn = Object.keys(events)
    .filter(key => !listeners[key] && typeof nextProps[key] === 'function')
    .reduce((acc, next) => ((acc[next] = events[next]), acc), {});

  const newListeners = listenEvents(toListenOn, nextProps, map);

  return { ...listeners, ...newListeners };
};

It:

  1. Checks which events we should stop listen to (toListenOff variable)
  2. Stops listen to these events (toListenOff.forEach)
  3. Checks which events should be listened to (toListenOn variable)
  4. Recreate the listener to these events with the new handlers (function called when event is emitted) (call to listenEvents function)

I think that this check typeof nextProps[eventKey] !== 'function' causes the bug (first line of code).
I tried with typeof nextProps[eventKey] === 'function' and it worked.
toListenOff is now equal to { onClick: 'click' }, it's listened off then listened on and my console.log(count) now displays 1

@sanfilippopablo
Copy link
Contributor

sanfilippopablo commented Jan 17, 2020

@qlerebours You are correct, that's the problem. The solution is a little bit off, tho. The code right now checks:

  • If there was a listener and got removed, then unlisten from the map.
  • If there wasn't a listener before and one was added to the props, add it to the map.

But it's not checking if the listener simply changed.
Pretty sure it's the same reason on #801 .
I just pushed a fix on PR #820 .

@ebrelsford
Copy link

This is an issue for me, too, as it makes it difficult to use hooks and accommodate more complex interactions with maps.

@crosskpixel
Copy link
Author

@ebrelsford I used the qlerebours solution, for me it worked well for now.

@ebrelsford
Copy link

I can also use the ref workaround but that doesn't seem ideal, right?

@olso
Copy link

olso commented Apr 16, 2020

Since, it's still not merged, I have

// https://github.com/alex3165/react-mapbox-gl/issues/748#issuecomment-569395282
  const _WORKAROUND_showList = React.useRef(true);
  const [, forceUpdate] = React.useState();
  // const [showList, setShowList] = React.useState(true);

// and

const handleMapClick = React.useCallback<MapEvent>(() => {
    closePopup();
    _WORKAROUND_showList.current = !_WORKAROUND_showList.current;
    forceUpdate({});
    // setShowList(!showList);
  }, [
    // setShowList,
    // showList,
    forceUpdate,
    closePopup,
  ]);

@dimagimburg
Copy link

dimagimburg commented May 18, 2020

Any updates on this? right now im using the forked version of @sanfilippopablo

@mklopets
Copy link
Collaborator

Should be fixed in the latest version, thanks to the PR by @sanfilippopablo. If true, let's close this.

@dimagimburg
Copy link

@mklopets Looks good to me :) can you please also publish it to npm registry so I can use my default package.json? Thanks a lot!

@mklopets
Copy link
Collaborator

Sorry, thought I did 🤦 now it's properly published in 4.8.6.

@nilquera
Copy link

I think these changes are not in the current npm release. Browsing the master branch commits, it seems like the PR changes were undone. Does anyone know why?

@prototypersf
Copy link

Looks like the issue is still happening in the mapboxgl v4. Any work arounds to fix it?

@goldensunliu
Copy link

onClick does not see state hooks changes #963

try #963 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests