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

Crash when removing <RFeature> from <RLayerVector> #1

Closed
jonas-peeters opened this issue Jun 1, 2021 · 9 comments
Closed

Crash when removing <RFeature> from <RLayerVector> #1

jonas-peeters opened this issue Jun 1, 2021 · 9 comments

Comments

@jonas-peeters
Copy link

jonas-peeters commented Jun 1, 2021

Hello,

I have a data array that is mapped to <RFeature> components inside an RLayerVector component.

Everything is working perfectly until I remove objects from my data array. At this point the application crashes and I get the following error message: Uncaught DOMException: Node.removeChild: The node to be removed is not a child of this node

From what I can tell, this has to do with DOM manipulation from either rlayers or ol. Any advice on how to solve the problem?

Edit: GitHub removes HTML-tag like strings when they are not in a code block

@mmomtchev
Copy link
Owner

Can you post a reproduction or a backtrace?

@mmomtchev
Copy link
Owner

I added an example for adding & deleting features at https://mmomtchev.github.io/rlayers/#/add_delete
Check it to see if it covers your case, there is a common pitfall with the React key field - it must stay the same for all RFeature that have not been deleted - if you use the array index it won't work.

@jonas-peeters
Copy link
Author

Thank you very much for the quick response.

I have tested your example (it works for me as well) and discovered that the issue only occurs when using an ROverlay inside the RFeature. Regardless of the ROverlay's children the application crashes in this case.

I created a minimal example in a repository here: https://github.com/jonas-peeters/rlayer-test
The relevant code is in this file at the lines 41 and 80: https://github.com/jonas-peeters/rlayer-test/blob/master/src/App.tsx
A production build is in the '/docs' folder and published using Github Pages here: https://rlayer-test.github.peeters.page/

Here are the two error messages from the development build:

Uncaught DOMException: Node.removeChild: The node to be removed is not a child of this node react-dom.development.js:10301
    React 8
    commitRootImpl self-hosted:1277
    unstable_runWithPriority scheduler.development.js:468
    React 3
    performSyncWorkOnRoot self-hosted:1220
    flushSyncCallbackQueueImpl React
    unstable_runWithPriority scheduler.development.js:468
    React 6
    bind_applyFunctionN self-hosted:1371
    dispatchDiscreteEvent self-hosted:1334
The above error occurred in the <RFeature> component:

RFeature@http://localhost:3000/static/js/vendors~main.chunk.js:132110:24
RLayerVector@http://localhost:3000/static/js/vendors~main.chunk.js:136795:24
div
RMap@http://localhost:3000/static/js/vendors~main.chunk.js:132509:24
App@http://localhost:3000/static/js/main.chunk.js:162:81

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries. index.js:1
    e index.js:1
    overrideMethod react_devtools_backend.js:2560
    React 10
    unstable_runWithPriority scheduler.development.js:468
    React 4
    unstable_runWithPriority scheduler.development.js:468
    React 6
        runWithPriority$1
        flushSyncCallbackQueueImpl
        flushSyncCallbackQueue
        discreteUpdates$1
        discreteUpdates
        dispatchDiscreteEvent

@mmomtchev
Copy link
Owner

Yes, indeed, this is a real problem. React doesn't detect that RFeature has disappeared from the DOM as RFeature does not have DOM-visible elements. Adding a DOM-visible element to RFeature solves the issue, I will check that it doesn't break anything else and I will release a new version.

@mmomtchev
Copy link
Owner

@jonas-peeters I released 1.0.2, thanks for reporting the problem

@jonas-peeters
Copy link
Author

I have tested the new version and it works great. Thank you very much.

@stefgootzen
Copy link

I've been struggling with a variation of this. I've adjusted the above reproduction with this case (and RLayers 1.5.2): https://github.com/stefgootzen/rlayers-reproduction

I get the same error when only removing the ROverlay, like:

{
  features.map(f => <RFeature key={f.id} geometry={new Point(fromLonLat([f.longitude, f.latitude]))}>
    {/*
            Adding an ROverlay to a feature will cause a crash when the overlay is removed
        */}
    {
      (f.id === activeFeatureId) && (
        <ROverlay key={f.id}>
          <div>{f.id}</div>
        </ROverlay>
      )
    }
  </RFeature>)
}

(Relevant file here)

Any thoughts on how to fix this? Thanks in advance!

@mmomtchev
Copy link
Owner

Change your overlay to:

{f.id === activeFeatureId && (
    <div>
        <ROverlay>
            <div>{f.id}</div>
        </ROverlay>
    </div>
)}

OpenLayers is removing the containing <div> of the overlay and this is confusing React.

@stefgootzen
Copy link

Awesome, many thanks. Works like a charm.

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

3 participants