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

Cannot read property 'matches' of null #666

Closed
vdh opened this issue Oct 18, 2017 · 9 comments
Closed

Cannot read property 'matches' of null #666

vdh opened this issue Oct 18, 2017 · 9 comments

Comments

@vdh
Copy link
Contributor

vdh commented Oct 18, 2017

Due to the addition of dom-helpers/query/matches in src/utils/interaction.js it's caused this race condition error:

Uncaught TypeError: Cannot read property 'matches' of null

Which appears to be this dom-helpers issue: react-bootstrap/dom-helpers#29

@vdh
Copy link
Contributor Author

vdh commented Oct 18, 2017

After further investigation, this also breaks the Multiselect component past v4.0.0-rc.8 due to the import of dom-helpers/src/query/closest (which depends on matches).

@jquense
Copy link
Owner

jquense commented Oct 18, 2017

Load your scripts below the the body not in the head :)

@vdh
Copy link
Contributor Author

vdh commented Oct 19, 2017

@jquense So you're just going to bake this "asynchronous code dependencies implemented inside synchronous code dependencies" problem into the loading requirements of the entire react-widgets library? Is this requirement documented anywhere because it's alarming to suddenly find undocumented race conditions like this during loading.

This requirement will trickle on into forcing the entire Webpack loader to be forced down below the body, which I can't do because I have a bunch of horrible legacy code that I need to inject localisation code for ASAP, before some cowboy coder's inline script tags start firing.

This kind of "undocumented dependencies that are just assumed to be there already immediately during first execution" coding style is exactly what forced me to need to move all my script tags up to the top of the head in the first place!

@jquense
Copy link
Owner

jquense commented Oct 19, 2017

I understand you have issues here. I'm happy to take PRs. The problem is not async code nor a race condition. it's that the document.body needs to exist in order to feature detect DOM functionality.

@vdh
Copy link
Contributor Author

vdh commented Oct 20, 2017

@jquense But it is asynchronous code, DOMContentLoaded is an event, and document.body is null before it fires.

From Wikipedia:

A race condition or race hazard is the behavior of an electronic, software, or other system where the output is dependent on the sequence or timing of other uncontrollable events. It becomes a bug when events do not happen in the order the programmer intended.

I understand that it's best practises to put scripts below the body. If I could do it, I would. I understand that most people probably don't encounter this issue, because they would be optimising their code and loading it after the body… but it's absolutely a textbook definition of a race condition.

@vdh
Copy link
Contributor Author

vdh commented Oct 20, 2017

@jquense PR created: react-bootstrap/dom-helpers#33

@skulden13
Copy link

Hi @jquense,

I noticed that you've approved the PR react-bootstrap/dom-helpers#33 from @vdh. Thank you both!

Could you please also update the npm package in order to use the latest version of this package in react-widgets?

Thanks in advance, Denis

@seanpaulkelley
Copy link

I am unable to use the current version of this package available via
yarn add react-widgets
do to this issue's error. From the comment by @skulden13 it looks like this is fixed in master branch. I would love to try this fix out. I am not entirely sure how to do this until package is updated.

I am new to using package managers. If I try to add this via
yarn add https://github.com/jquense/react-widgets.git
I get error
Package "undefined@undefined" doesn't have a "name".

I posted a general question of use of package managers here because I am not sure if this is an issue with this repository or my use of yarn

@jquense jquense closed this as completed Feb 8, 2018
@skulden13
Copy link

skulden13 commented Feb 8, 2018

Hey @jquense,

I noticed that you've updated dom-helpers and bumped the version of react-widgets.
Thanks for your update! 🎉

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

4 participants