brentd / xray-rails

☠️ A development tool that reveals your UI's bones
MIT License
1.22k stars 79 forks source link

Add config option to defer loading of Xray script #98

Open SDRACK opened 5 years ago

SDRACK commented 5 years ago

When loading of jQuery is deferred, the Javascript Xray script loads ahead of this and loads a blank object.

This is a stripped down approach to allow defer loading the script, via a config option.

mattbrictson commented 5 years ago

Thanks for the PR. I don't think this is the right approach because local_config is for configuration that is developer-specific, not project-specific (e.g. their preferred code editor). Reusing local_config for the purpose of storing the defer setting isn't quite right because defer needs to be a project-based setting, not something a developer sets in their ~/.xrayconfig.

I like the idea of configuring the defer attribute though. Is there a different way to do it? Maybe the app could have an initializer like config/initializers/xray.rb and in that file you set an attribute on the Xray object? Just thinking out loud, not sure if that is ideal either.

SDRACK commented 5 years ago

Thanks for the reply @mattbrictson, and apologies for the delay.

You're entirely right on this point, and, with that in mind, I'd agree an initializer would be the way to go.

When I get the time I'll update to reflect this. Once again, thanks for the feedback 👍

gfauredumont commented 5 years ago

I'm interested in this feature ;)

TylerRick commented 3 years ago

Technically, couldn't the middleware just check if there was a defer attribute in the script tag for jquery.js, and if so, copy its value to the injected script for xray.js?

I'm hesitant to even mention that, though, because now that I've thought more about it, I think the auto-adding the script feature is overrated, over-complicated, and error-prone (#100), and ought to just be removed (#110).