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

How to make sure component cleanup happens when no longer in use? #35

Closed
zapbampow opened this issue Apr 18, 2019 · 10 comments
Closed

How to make sure component cleanup happens when no longer in use? #35

zapbampow opened this issue Apr 18, 2019 · 10 comments
Labels
status:confirmed An issue confirmed by the development team. type:bug A bug.

Comments

@zapbampow
Copy link

zapbampow commented Apr 18, 2019

I'm having what I believe is a cleanup problem with my editor react component.

Here is a simplified version of my code and what is going on.

<App>
    <Switch>
        <Route path="/dashboard" component={Dashboard} />
        <Route path="/editor component={Editor} />
    </Switch>
</App>

I have a react-router <Switch /> component that will show either <Dashboard /> or <Editor />.

<Editor /> is a form, which includes <CKEditor />.

Finally, I can moved directly between the Dashboard to the Editor .

Here is what I'm running into. If I go to <Editor />, then to <Dashboard />, and back to <Editor />, then CKEditor fails to load and the whole page stalls. As far as I can tell, the original instance is still in the DOM and it is trying to create a new instance with the same name. At least that's what I'm guessing is going on. It could certainly be something else.

I'm getting TypeErrors that a is null and this.editor is null.

I'm trying to figure out how to make sure that the first instance of <CKEditor /> is removed so that it can load again on the page when I return to the <Editor /> component.

@f1ames
Copy link
Contributor

f1ames commented Apr 24, 2019

Hi @zapbampow,
have you figured out any solution for you issue? If so can you share it here as it might be useful for others?

@zapbampow
Copy link
Author

@f1ames, I thought I had found a solution by placing it inside a container component. I posted it here and closed the issue. Soon after that I realized the problem wasn't actually solved. So I deleted my solution, but couldn't reopen or delete the issue. I also couldn't reproduce the issue in a simplified app. My current solution is to live with it and manually reload the page after navigating <Editor /> a second time.

@f1ames
Copy link
Contributor

f1ames commented Apr 25, 2019

Thanks for the insights @zapbampow. I have reopened the issue, so we know it is not solved yet.

@f1ames f1ames reopened this Apr 25, 2019
@f1ames
Copy link
Contributor

f1ames commented Apr 25, 2019

By the way @zapbampow, would you be able to provide for example a simple codepen which reproduces this issue so we may take closer look?

@f1ames
Copy link
Contributor

f1ames commented Jul 12, 2019

I think this may get fixed after solving #44 (and merging #16).

@f1ames
Copy link
Contributor

f1ames commented Oct 15, 2019

Hello @zapbampow,
recently we have released stable 1.0.0 package which also utilizes a fix for #44 which should fix the issue you initially reported here.

Could you check if the issue is indeed fixed (you need CKEditor 4.13 which is loaded by default by React 1.0.0 integration)?

@zapbampow
Copy link
Author

It seems this does fix the problem. I used to get 2 errors. While I do still get the following error, it no longer crashes anything, which is an acceptable fix and my users will be happy.

image

@f1ames
Copy link
Contributor

f1ames commented Oct 16, 2019

Thanks @zapbampow for rechecking this issue. Since you still get an error, I will leave this issue open so we may get back to it and check what's going on 👍

@jacekbogdanski jacekbogdanski added type:bug A bug. status:confirmed An issue confirmed by the development team. and removed bug labels Feb 7, 2020
@f1ames
Copy link
Contributor

f1ames commented Jul 22, 2020

@zapbampow could you take a look if #91 fixes your issue (it's already merged to master)?

@sculpt0r
Copy link
Contributor

sculpt0r commented Jan 4, 2021

Hi @zapbampow, It's been a while since we last heard from you. We're closing this issue for now. Still, feel free to provide us requested feedback, so that we can reopen this issue.

@sculpt0r sculpt0r closed this as completed Jan 4, 2021
@f1ames f1ames removed this from the Backlog milestone Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

4 participants