mmomtchev / rlayers

React Component Library for OpenLayers
ISC License
173 stars 36 forks source link

Uncaught DOMException when using RControl.RCustom component #280

Closed sweco-nlgucu closed 2 months ago

sweco-nlgucu commented 3 months ago

Hi there,

we're encountering an Uncaught DOMException in a very specific situation where we have a changing amount of layers, in combination when a RControl.RCustom is also in the map.

The error: Uncaught DOMException: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.

It took me a bit to isolate the issue, as we're building a larger application than just this example. The example is trivial, but triggers the same error as we have in our app. In our own application this happens because we have to load the configuration of layers somewhere from an endpoint and a user will be able to add layers themselves. I've simulated that below with the useState and useEffect combination. This piece of App.jsx code shows the error, with pointers on how to work around and avoid the error:

import { useState, useEffect } from "react";
import { RControl, RMap, RLayerVector, ROSM } from "rlayers";
import { fromLonLat } from "ol/proj";
import "./App.css";

const coords = {
  origin: [2.364, 48.82],
  ArcDeTriomphe: [2.295, 48.8737],
};

// There's three solutions to preventing the error.
function App() {
  const [anArray, setAnArray] = useState([1, 2]);

  // Solution #1: remove this useEffect...
  useEffect(() => {
    setAnArray([1, 2, 3]);
  }, []);

  return (
    <RMap
      className="example-map"
      initial={{ center: fromLonLat(coords.origin), zoom: 11 }}
    >
      {/* Solution #2: move this ROSM component between the closing of the next fragment and the RControl component... */}
      <ROSM />
      <>
        {anArray.map((value) => (
          <RLayerVector key={value} />
        ))}
      </>
      {/* Solution #3: remove this RControl.RCustom */}
      <RControl.RCustom></RControl.RCustom>
    </RMap>
  );
}

export default App;

I can reproduce this error with at least rlayers versions 2.1.0 and 3.1.0

To try to reproduce it, please use this repo: https://github.com/sweco-nlgucu/rlayers-bug

I hope this description helps. If you need anything else, let me know! We can work around the issue by making sure there's an empty layer or something between that "array" of layers and the rcontrol.rcustom (similar to solution 2). But I do think this issue is a bug and unexpected behavior.

Cheers, Guus

mmomtchev commented 2 months ago

Is this something you actually need or your only goal was to make it crash?

mmomtchev commented 2 months ago

Put all of your controls in a separate <div> when using dynamic layers and it won't crash. This is too complex to fix given its benefit.

function App() {
    const [anArray, setAnArray] = useState([1, 2]);

    useEffect(() => {
        setAnArray([1, 2, 3]);
    }, []);

    return (
        <RMap className='example-map' initial={{center: fromLonLat(coords.origin), zoom: 11}}>
            <ROSM />
            <>
                {anArray.map((value) => (
                    <RLayerVector key={value} />
                ))}
            </>
            <div>
                <RControl.RCustom></RControl.RCustom>
            </div>
        </RMap>
    );
}
sweco-nlgucu commented 1 month ago

Is this something you actually need or your only goal was to make it crash?

Hi, thanks for looking into this.

Unfortunately I don't have the luxury of time at my work to see if something can crash. Like many of us we need to deliver features and this one appeared when I tried to implement one.

sweco-nlgucu commented 1 month ago

Too bad it's too complex to fix, but thanks for the suggested workaround. We'll create something like a rcustom container component with just the div and maybe some linting rule to avoid using the rcustom components directly in the map container.