OpenNews/opennews-source

JavaScript issue on staging?

beep opened this issue · 11 comments

beep commented

Seeing a really weird bug on most staging server URLs:

17:16:07.741 jQuery.Deferred exception: Argument 1 of Window.getComputedStyle is not an object. _isNonInteractive@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:26799
_checkInteractivity@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:27046
_bindEvents@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:27383
init@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:26157
r@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:25620
e.fn[a]/<@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:28283
each@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:1:14861
each@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:1:13031
e.fn[a]@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:28245
@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:28344
dispatch@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:18828
add/A.handle@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:16882
trigger@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:8099
trigger/<@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:8613
each@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:1:14861
each@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:1:13031
trigger@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:3:8592
@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:4:27511
u@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:9226
a/</f<@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:9543
setTimeout handler*a/<@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:9754
d@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:7464
fireWith@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:8240
fire@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:8274
d@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:7464
fireWith@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:8240
ready@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:11245
setTimeout handler*@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:2:11379
@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:1:220
@https://opennews-source-staging.herokuapp.com/static/base/_v2/dist/js/main.js:1:2
 undefined 1 main.js:2:10821

Never seen anything quite like this. It seems to be preventing the JS from fully executing, which is causing some JS/errors. And I can’t replicate that locally! (On the static templates, anyway.)

(Noticed on #43, ping)

I'm hoping that pushing the latest static file updates to staging will resolve this? I'm not actually seeing the error anywhere. Heroku currently having some build issues though, so I'll push once those are clear. https://status.heroku.com/incidents/1041

@beep So the build problem is resolved and staging is updated, but I'm seeing the same issue there as well as locally. I haven't ever run into this one before either. A bit of quick research yielded a couple maybe useful hints:

It feels like this is a jquery problem, like it's trying to act on an element that isn't available yet? I'm totally guessing here, trying to figure out why it might not work on these pages but it does work on your static pages.

beep commented

like it's trying to act on an element that isn't available yet?

Yep, that’s about where I’d gotten—there’s definitely a timing issue happening here.

I’ll see if I can narrow down a test case.

beep commented

Hey, @ryanpitts, good morning, I have two questions for you!

  1. How involved would it be for me to get the django app running locally?
  2. Is it possible to inline initial.js in the template, rather than linking to it via script?
beep commented

Okay, the increasingly poorly-named #56 has a fix that should fix this. This bug snuck in after #51 landed; I forgot to catch a markup change that was needed in base.html—sorry about that, folks.

oh nice! Sorry about the Pacific Time lag here, but that's awesome. Re: the two questions you posted:

  1. If you're marginally comfortable with Python, we could totally do that. The biggest issue would be setting up the test database, which currently involves a fun time with postgresql. I could look into making a sqlite test db if this feels like a useful thing though.
  2. Django's template include for inlining objects works a bit differently than PHP's include from what I can tell -- the most important difference being that Django verifies that you're including a template-like object, and straight javascript files don't seem to work. (I'd never tried doing this before, but I banged around on it last week, and at least at that point this was the conclusion I reached.) It would still be possible to inline initial.js by making an initial.js.html template for Django purposes, though. Or I might be able to find another solution. I set that aside in favor of just linking via script tag to get things moving, and planned to ask how important inlining might be here.
beep commented
  1. I think since this was 100% an Ethan-Induced Bug™©®, we can safely skip this for now. But thank you!
  2. It doesn’t seem related to this bug, but inlining the JS (and, if we can use it, the critical CSS) would be great from a performance standpoint, since script / link requests will block rendering. Not a critical thing at this point, mind! But maybe something to track.

(Any chance this would help? (Also: I know nothing about Django hi))

Yep, django-compressor is a good solution, and up until this refactor, Source got to use it :) A lot of django-compressor things don't work right on Heroku, though (I think because you can't write to the local filesystem).

I think I might be able to get around this with some post_compile scripting, but I've been sidelining a few performance-related things like that.

Seriously, getting to see how you organize front-end projects and approach best practices for performance has been one of the best parts of this project for me.

beep commented

Yep, django-compressor is a good solution, and up until this refactor, Source got to use it :)

Oh, ha, gotcha!

Seriously, getting to see how you organize front-end projects and approach best practices for performance has been one of the best parts of this project for me.

That’s incredibly kind of you, @ryanpitts—thank you. Most of what I’ve done is cribbed from the work of Filament Group and others; happy to walk through what’s there in more detail at any point.

I'm pretty sure this is entirely cleared up now, but I'll let you verify before closing ...

beep commented

This looks good to me. Yay!