I'm voting for only inlcuding the JS libraries, if getDrupalSettingCommands() returns any result. Because now, even if you don't set an Analytics ID and also turn off the config setting to add default commands and page view, and therefore no command is being returned, the analytics.js script gets loaded.

We shouldn't load any library at all, if this is the case

CommentFileSizeAuthor
#2 3069900-2.patch698 bytesagoradesign

Comments

agoradesign created an issue. See original summary.

agoradesign’s picture

Status: Active » Needs review
StatusFileSize
new698 bytes
gapple’s picture

It's possible, but probably unlikely, for someone to use the module for including the analytics script but not using the collect event to set commands. I think in that case they should just be sure to attach the library to the page themselves to ensure it's present.

  • gapple committed e1c4a85 on 8.x-1.x
    Issue #3069900 by agoradesign, gapple: Only attach library to page if...
gapple’s picture

Status: Needs review » Fixed

Committed with one change

- getDrupalSettingCommands() was called twice instead of re-using the result from the first call

agoradesign’s picture

Thanks! :-)

regarding #3 - we tend to install ga in every project, because in the end, nearly every site owner wants/needs GA. We rather go for uninstalling it later, if it's really not needed. But if you can mute the module without having any library attached, it's no big harm to keep it enabled. Also, we're disabling the config settings via override in dev/test/staging environments. I've just recognized that.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.