tether icon indicating copy to clipboard operation
tether copied to clipboard

getScrollParents util can push "null" as sole scroll parent when body has not been rendered, causing fatal error

Open circAssimilate opened this issue 6 years ago • 7 comments

getScrollParents util can push null as sole scroll parent, causing a fatal error, when the body has not been rendered. This is the line that is leading to the below error:

https://github.com/HubSpot/tether/blob/1fea67260ebdd6bc3d42ebe7f91b367174178d97/src/js/utils.js#L65

Getting Uncaught TypeError: Cannot read property 'addEventListener' of null

In our application, we load our main bundle before the body for performance reasons. Is there a possibility for an option that can delay tethering until the body is ready? Open to any other approaches too.

circAssimilate avatar Apr 24 '18 17:04 circAssimilate

Perf reasons? Actually all the best practices say to load js after the body

FezVrasta avatar Apr 25 '18 06:04 FezVrasta

I do not agree that it's a best practice to load all JS in the head, but I can understand if that's the requirements for Tether. I could see other users running into this fwiw.

In the case that this is expected, are there any existing options available to defer the creation until an element (or specifically the body element) exists or should we do that on our end before creating a new Tether (e.g. via polling or a Mutation Observer waiting for the body)?

circAssimilate avatar Apr 25 '18 17:04 circAssimilate

Nope. It's best practice to load it after the body.

Anyway this library is in maintenance mode, if you need a more up to date library use Popper.js as suggested in the readme

FezVrasta avatar Apr 26 '18 07:04 FezVrasta

Nope. It's best practice to load it after the body.

Theres no such thing as after the body. I think you mean inside the

Actually all the best practices say to load js after the body

This is not true at all. I'll cite Google's recommendation which is focused on blocking scripts, as well as React's getting started documentation as cookie cutter examples of loading JS in the

It's perfectly legitimate to place javascript tags in the

to begin fetching them as soon as possible. If a script is cached, its completely possible for it to run before the is defined.

The issue the OP is running into is the Tether constructor being invoked before the

has been rendered into the DOM.

At a minimum this behavior should be documented. More so, the code shouldn't assume that the

exists when it runs and handle that case.

jamesopti avatar Apr 26 '18 18:04 jamesopti

Apologies for the snarky tone in my reply!

What I mean to convey is that the author of any javascript bundle should be aware of what dependencies it has on DOM state.

If you choose to load JS before the

is defined, you better make sure you wait for or whatever element you're mounting to before calling ReactDOM.render

However because React (like other UI frameworks) allows for components to be created and mounted before being actually inserted into the DOM, any 3rd party library should be clear about it's own dependency on DOM state that is not built into React itself.

Also thanks for the pointer @FezVrasta to this repo being in maintenance. The OP should probably just migrate to Popper as suggested.

jamesopti avatar Apr 26 '18 18:04 jamesopti

I can definitely see a clear use case where this wouldn't create a Tether until BODY was available. I'd be open to either a polling function or Mutation Observer option as noted in https://github.com/shipshapecode/tether/issues/285#issuecomment-384367777. Nice solution and suggestion @circAssimilate.

Original location of issue has moved to it's own utils module and is here: https://github.com/shipshapecode/tether/blob/8375ee43d9f48460611b0f354ffe7488224f0644/src/js/utils/parents.js#L36

chuckcarpenter avatar Oct 01 '19 20:10 chuckcarpenter

@chuckcarpenter wouldn't it be better to handle this in the consuming app? Wait until the element you want to tether to exists before creating the tether?

RobbieTheWagner avatar Oct 01 '19 20:10 RobbieTheWagner