jccartwright / exb-widgets

sandbox for custom widgets to be used in ArcGIS Experience Builder
0 stars 1 forks source link

state variables not accessible within nested functions #23

Closed jccartwright closed 2 years ago

jccartwright commented 2 years ago

in the h3-layer Widget state variables, e.g. 'h3', are available at the top-level of the widget but not within directly nested functions, e.g. mapClickHandler. A similar problem exists with variables, e.g. widgetState defined at the top-level of the widget function but inaccessible within a directly nested function, e.g. hexbinSummary. Curiously, both of these example variables are visible within a useEffect function.

Workaround seems to be assigning the value to a useRef but not clear why this is necessary.

shawnmgoulet commented 2 years ago

@jccartwright so the issue mainly stems from not including the h3 state property within a useEffect function.

To remove the need to associate a useRef property with the h3 useState value:

  1. Remove const prevH3 = useRef()
  2. Remove prevH3.current = h3 Within either useEffect function, add , [h3] after the useEffect's callback:
    useEffect(() => {
    console.log(`inside useEffect: h3 = ${h3}; queryParams = ${widgetState?.queryParams}`)
    }**, [h3]**)

    👆 this is the key, as you'll get a re-render every time the h3 useState prop is changed, thereby updating the render of it.

  3. Replace: if (graphicHits?.length === 1 && graphicHits[0].graphic.attributes.h3 !== prevH3.current) { with: if (graphicHits?.length === 1) {

To restate, you'll no longer need to use the prevH3 useRef property. If you wanted to render/update the pointCount useState property, you would follow this same pattern.

https://user-images.githubusercontent.com/3179974/195664925-df409b29-d181-4f4e-a30c-3ea1fd1fb480.mp4

shawnmgoulet commented 2 years ago

To explain retrieving the setState properties/values within other functions, for this case, it is recommended to retrieve them via the useEffect.

So, within the mapClickHandler, even though highlightHexbin sets the h3 property's value, setState is asynchronous, therefore does not happen immediately. Therefore, if we want to access what h3 is after highlightHexbin > setH3, we need to refer to graphicHits[0].graphic.attributes.h3 within the if (graphicHits?.length === 1) { condition.

Within activeViewChangeHandler, it's recommended to remove the .then(async () => { updateGraphicsLayer()...}). Move the updateGraphicsLayer() execution into a useEffect like:

  useEffect(() => {
    if (h3 !== null) {
      updateGraphicsLayer()
    }
  }, [h3])

therefore only running if the h3 state value isn't null. Also, if you want to add additional logic to that like what is currently commented out in the .then callback of the Promise (standing up a new FeatureLayer, etc.), you will also want to set a JimuMapView property/value prior and add that to your useEffect qualifier above - with the components needed like:

// add to consts
 const [jimuMapView, setJimuMapView] = useState<JimuMapView>(null);

// add to jmv.view.when()
setJimuMapView(jmv);

// add jimuMapView to useEffect
  useEffect(() => {
    if (h3 !== null && jimuMapView !== null) {
      updateGraphicsLayer()
    }
  }, [h3, jimuMapView]);
jccartwright commented 2 years ago

Thanks @shawnmgoulet. I didn't see that any of the above directly addressed the question of accessing the state w/in the event handlers but your suggestion of abstracting the business logic of the event handler to a useEffect nicely sidesteps the problem and makes the code cleaner anyway.

Although there are still aspects of this issue I don't understand, they've become more matters of curiosity and don't really justify more time on either of our parts so I'm closing the issue.

I would be grateful if you could take another quick review of the current code to see what you think.

shawnmgoulet commented 2 years ago

@jccartwright In this case, the reason you cannot access a state prop/value is likely due to a stale setState, likely due to the logic being bunched up in locations, especially within the mapClickHandler. That is why I recommended pulling pieces apart as you could (e.g., doing something with a FeatureLayer).

This CodeSandbox is a simpler example of a stale setState within the handleButtonClicked > delay.then() method.

In your case, you can move logic to the useEffect and there's nothing wrong in doing so. It's a different approach, still very much valid and could be viewed as more appropriate in your situation. Separating logic as much as possible is best.

shawnmgoulet commented 2 years ago

@jccartwright

This looks good to me. Nicely done.

The 2 useEffects on the widgetState.queryParams (to account for query changes) and selectedGraphic to ultimately update the depth range & phylum counts make sense to me.

Removing h3/selectedGraphic from directly from the useState as previous and into the useEffect seems to work nicely. Again, something seemed to be stale with it.