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

TypeError: Cannot read property 'insertBefore' of null and Cannot set property 'isReadOnly' of null #83

Closed
thtg88 opened this issue Mar 26, 2019 · 19 comments
Assignees
Labels
Milestone

Comments

@thtg88
Copy link

thtg88 commented Mar 26, 2019

Hi guys, I'm trying to use your CKEditor 5 React component in a Create React App v2, to build a big form, together with other standard HTML form inputs, and other instances of CKEditor all in the same form.
The form itself is driven by a HOC with a data structure which drives all of the form state, something in the lines of:

state = {
  resource: {
    name: {
      value: '',
      name: 'name',
      type: 'text',
    },
    content: {
      value: '',
      name: 'content',
      type: 'ckeditor',
    }
    parent_id: {
      value: '',
      name: 'parent_id',
      type: 'select',
      // This gets populating by data-fetching from an external REST API
      values: [],
    }
  }
}

Part of this state/schema waits for additional values to be updated after a fetch call is fired to a REST API, basically in order to populate select options and values.
This causes a few setStates right at the moment of fetch (to update UI state with spinners), and at the moment of receiving back those values (simply updating the state with them).
While this works seamlessly with other form inputs, it looks like CKEditor throws an error if there are too many setStates close to each other, it looks like the this.editor instance in the CKEditor component is not refreshed in time and a TypeError: Cannot read property 'insertBefore' of null is thrown.

I've created a CodeSandbox for you to try, while it is not exactly the same code I'm using (the CodeSandbox leverages hooks, and setInterval for firing off quick and close to each other setState calls), it can hopefully showcase the problem.
https://codesandbox.io/s/py52ll3v4j?fontsize=14

Additional to that (this I can't really reproduce on CodeSandbox unfortunately), CKEditor throws another error on my CRA when the disabled prop is passed to the CKEditor component and these repeated setState calls throw the error TypeError: Cannot set property 'isReadOnly' of null
Which seems to occur since the latest v1.1.2 (was not occurring on v1.1.1) due to this edit in shouldComponentUpdate.

Could you point me in the right direction if I'm doing something wrong in the way I'm using the component or if there's an issue with CKEditor itself?

Thanks guys,
Marco

@pomek pomek added the pending:feedback This issue is blocked by necessary feedback. label Mar 26, 2019
@pomek
Copy link
Member

pomek commented Mar 26, 2019

Hi Marco,

I was trying to reproduce the first issue that you described using your sample. Unfortunately, it always works. I tried to refresh the page a few times but the dev-console is always clear. Any tip that could help reproduce the issue would be appreciated.

For the second issue – I'll try to write a test case that will call setState() a few time. If I will be able to see the attached error, I will fix it.

@thtg88
Copy link
Author

thtg88 commented Mar 26, 2019

Hi Kamil, thanks for your quick reply. I tried again on my phone and you're right, now the sandbox always seems to work 😥 I'll try to make a more reliable example, and I'll get back to you, thanks again in the meantime :-)

@thtg88
Copy link
Author

thtg88 commented Mar 27, 2019

Hi Kamil, I couldn't really reproduce this on CodeSandbox unfortunately, but I've recreated a minimal viable example on a private GitHub repo. I sent you an invitation to collaborate, can you let me know if it makes sense?
Thanks,
Marco

@pomek
Copy link
Member

pomek commented Mar 27, 2019

Is there any chance to make the sample publish? Other developers will be able to reproduce the issue. If more people will be able to check this out, it's more probable that the issue will be fixed.

@thtg88
Copy link
Author

thtg88 commented Mar 27, 2019

Hi Kamil, I've published it for you at https://github.com/thtg88/ckeditor5-react-re-render-error, you may want to re-clone if you have cloned previously as I've done a few changes to reduce code.
Let me know if you need any additional info.
Thanks,
Marco

@pomek
Copy link
Member

pomek commented Mar 27, 2019

Huge thanks for preparing the entire environment. I'll back to you this week with some update if I'll anything know about the bug.

@thtg88
Copy link
Author

thtg88 commented Mar 27, 2019

No worries thank you guys for all the work you put in this - hope this time the error shows :P

@pomek
Copy link
Member

pomek commented Mar 28, 2019

Seems like

shouldComponentUpdate( nextProps ) {
	if ( !this.editor ) {
		return false;
	}

	// ...
}

fixes an issue Cannot set property 'isReadOnly' of null.

Your application does not throw any other errors that you mentioned in the issue. Is the error: TypeError: Cannot read property 'insertBefore' of null still a problem?

@thtg88
Copy link
Author

thtg88 commented Mar 28, 2019

I haven't been able to isolate the insertBefore issue as yet, but I'm happy if the isReadOnly one has been solved (that's the more frequent one) :) do you think I should change the title of the issue just in case other people stumble on this?

@pomek pomek changed the title TypeError: Cannot read property 'insertBefore' of null TypeError: Cannot read property 'insertBefore' of null and Cannot set property 'isReadOnly' of null Mar 28, 2019
@pomek
Copy link
Member

pomek commented Mar 28, 2019

I did it :)

@thtg88
Copy link
Author

thtg88 commented Mar 28, 2019

Thanks Kamil, do you reckon this is going to be in a possible next patch release?

@pomek
Copy link
Member

pomek commented Mar 28, 2019

I am trying to write a test that covers my change. When I will be ready, I am going to create a PR. If everything will go smoothly and PR will be merged, I'll release a new version. It should be done next week. I am not sure whether releasing anything on Friday is a good idea :)

@pomek pomek added type:bug status:confirmed and removed pending:feedback This issue is blocked by necessary feedback. labels Mar 28, 2019
@pomek pomek self-assigned this Mar 28, 2019
@pomek pomek added this to the iteration 23 milestone Mar 28, 2019
@thtg88
Copy link
Author

thtg88 commented Mar 28, 2019

Never a good idea releasing on a Friday :D Thanks for the update and quick resolution!

@karuhanga
Copy link

karuhanga commented Mar 29, 2019

Seems like

shouldComponentUpdate( nextProps ) {
	if ( !this.editor ) {
		return false;
	}

	// ...
}

fixes an issue Cannot set property 'isReadOnly' of null.

Your application does not throw any other errors that you mentioned in the issue. Is the error: TypeError: Cannot read property 'insertBefore' of null still a problem?

I'm not sure if this is directly related, but I had an issue that seems like it might be related to this.
image

Doing this helped.

It might be worth pointing out that this only happens in the production build and does not show up in the development build.

@pomek
Copy link
Member

pomek commented Mar 29, 2019

Could I ask you about checking whether the proposed solution in https://github.com/ckeditor/ckeditor5-react/pull/85/files fixes your issue? If not, I will appreciate if you write an instruction step by step how to reproduce the mentioned issue.

At this moment I can't say your issue and @thtg88's are related to each other.

ma2ciek added a commit that referenced this issue Mar 29, 2019
Fix: The `<CKEditor>` component will not update anything until it is not ready. Closes #83.
@pomek
Copy link
Member

pomek commented Mar 29, 2019

@thtg88, if you will be able to reproduce the second issue, could you report it in a separate issue?

PS: You can hide the repository that reproduces the resolved issue. We don't need it anymore.

@thtg88
Copy link
Author

thtg88 commented Mar 29, 2019

Sure, no problem.
Thanks again guys!
Marco

@pomek
Copy link
Member

pomek commented Apr 1, 2019

@thtg88
Copy link
Author

thtg88 commented Apr 1, 2019

@pomek thank you very much - I can confirm this has fixed the isReadOnly issue 👍

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

No branches or pull requests

3 participants