Show a warning instead of throwing an error on invalid GTM ID

[altescape]

Info

App won't start if an invalid GTM ID is used in my setup (which, I believe, is a pretty standard setup).

Tool	Version
Plugin	v1.0.0 (Vue2)
Vue	v2.6.12
Node	v14.15.5
OS	Ubuntu 20.04.2 LTS

Input

import Vue from "vue";
import App from "./App.vue";
import VueGtm from "@gtm-support/vue2-gtm";

Vue.use(VueGtm, {
  id: "INVALID"
});

new Vue({
  render: (h) => h(App)
}).$mount("#app");

Output or Error

Crash screen with error report on codesandbox.io or Console.error and a blank screen in localhost:

assert-is-gtm-id.js:15 Uncaught Error: GTM-ID 'INVALID' is not valid
    at Object.assertIsGtmId (assert-is-gtm-id.js:15)
    at new GtmSupport (gtm-support.js:35)
    at Object.install (index.js:29)
    at Function.Vue.use (vue.esm.js:5116)
    at Module../src/main.js (main.js:37)
    at __webpack_require__ (bootstrap:769)
    at fn (bootstrap:129)
    at Object.0 (app.js:136081)
    at __webpack_require__ (bootstrap:769)
    at bootstrap:907

Expected Output

I would expect a console warning Your are using an invalid GTM ID and the app should load but without GTM.

Additional Context

TryCatch gets around the issue and so does using a valid GTM ID obviously ;-) but for those cases where an ID is fat fingered into an environment variable for example I would expect the plugin to catch and handle it's own error.

try {
  Vue.use(VueGtm, {
    id: 'INVALID',
    vueRouter: router
  })
} catch (error) {
  console.warn(error)
}

[Shinigami92]

Mh until now this was by design :thinking: Also seems you found a good workaround...

So don't know if I should just log a warning I even have tests that check that this error is thrown :eyes:

https://github.com/gtm-support/vue-gtm/blob/8d9956273a0b823411eaa9520b711aaed6449be3/tests/index.test.ts#L17-L39

[3askaal]

While implementing I came across this issue as well. Just wanted to point out that it would be way easier if it would just throw a warning and be disabled by default when you don't have a valid GTM-ID. So you don't have to add extra checks for environments where you might not have these values.

Besides that, nice job on maintaining this! :)

[JFGHT]

@Shinigami92, this should be solved by the library indeed, not by the dev who uses it, so it's incorrect by design.

[Shinigami92]

@Shinigami92, this should be solved by the library indeed, not by the dev who uses it, so it's incorrect by design.

Maybe I will have time in next few days to review a PR from you.

[JFGHT]

@Shinigami92, this should be solved by the library indeed, not by the dev who uses it, so it's incorrect by design.

Maybe I will have time in next few days to review a PR from you.

You are right on asking for help. I was just pointing out that the guys from above are also right and this Issue should not be closed, so it has visibility and other people can contribute.

[ST-DDT]

IMO it is correct to throw an error, if the id is syntactically invalid. For prod this is also the expected behavior. If it isn't configured (properly) then I want to get an explicit error, so that it is obvious (aka fail fast).

IMO the right solution for tests is using a syntactically correct but invalid id like GTM-INVALID. Maybe you can change the error message to account for that:

if (!/GTM-[0-9A-Z]+/.matches(id)) {
    throw new Error(`${ id } is not a valid GTM-ID (/GTM-[0-9A-Z]+/).
		Did you mean: 'GTM-${ id.toUpperCase().replace(/.*-|[^0-9A-Z]/g, '') }'?`);
}

[Shinigami92]

Yeah, I see it the same way It's an error by design and it is easily solvable via just adding e.g. GTM- before the INVALID resulting in GTM-INVALID

The maintenance burden that was raised by not having this validation and everyone coming and telling me that they GA-ID didn't work was much higher and would not be solved by a warning that could be just ignored as it shows silently in a console somewhere.

Done via https://github.com/gtm-support/core/commit/34d4eda7e376e82ffc744f24508959cd33aada84

[Jon78]

We recently switched to Google Analytics 4 and were provided with a new ID. This new ID starts with G-, in stead of GTM-. As a result our app breaks. The regexp should read something like /GTM-[0-9A-Z]+|G-[0-9A-Z]{10,}$/ Also this should really be a warning.