Add a method in the Google Analytics Provider to explicitly start tracking route changes

[rossedfort]

I'm submitting a ...

• bug report
• feature request
• question about the decisions made in the repository

---

Under step 2 of Include it in your application in the angulartics documentation, It says simply injecting the Angulartics2GoogleAnalytics provider in your root application component will start tracking route changes. This is true, it works fine, however, the approach goes against some general conventions. The documentation implies to inject the provider into the component, but forgo making it a member of any instances of the component class (implied by lack of private or public keyword in the constructor). This seems right, however, the value of the provider is never used in the constructor. This causes the TypeScript compiler to complain, if you have the following rule enabled:

tsconfig.json

"noUnusedLocals": true

export class AppComponent {
  constructor(angulartics: Angulartics2GoogleAnalytics) {} // not good, angulartics is never used in the constructor
}

---

Adding the public keyword to the injection, thus making the provider a public member on the instance of the component gets ride of the compiler error (because the compiler can't check if its value is ever used), but makes the code incorrect.

export class AppComponent {
  constructor(public angulartics: Angulartics2GoogleAnalytics) {} // not good, this.angulartics is not accessed by any other classes 
}

---

Adding the private keyword to the injection, thus making the provider a private member on the instance of the component seems more correct, yet again, causes a compiler error, as its value is never used

export class AppComponent {
  constructor(private angulartics: Angulartics2GoogleAnalytics) {} // not good, this.angulartics is not accessed within this class
}

---

My suggestion/request is to implement a method that users would call to explicitly initiate tracking. This ensures the code is conventional.

export class AppComponent {
  constructor(angulartics: Angulartics2GoogleAnalytics) {
    angulartics.startRouteTracking(); // good, angulartics is not a member on the instance, and its value is used in the constructor
  }
}

Unless I'm missing something, TSLint, the TypeScript compiler, and even Angular's Dependency Injection Documentation all indicate this as a code smell.

Thanks for reading, and thanks for this great library.

$ ./node_modules/.bin/ng --version

    _                      _                 ____ _     ___
   / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
  / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
 / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
/_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
               |___/

Angular CLI: 1.7.4
Node: 9.0.0
OS: darwin x64
Angular: 5.2.9
... animations, common, compiler, compiler-cli, core, forms
... http, platform-browser, platform-browser-dynamic
... platform-server, router

@angular/cdk: 5.2.4
@angular/cli: 1.7.4
@angular/material: 5.2.4
@angular-devkit/build-optimizer: 0.3.2
@angular-devkit/core: 0.3.2
@angular-devkit/schematics: 0.3.2
@ngtools/json-schema: 1.2.0
@ngtools/webpack: 1.10.2
@schematics/angular: 0.3.2
@schematics/package-update: 0.3.2
typescript: 2.6.2
webpack-bundle-analyzer: 2.11.3
webpack: 3.11.0

Using angulartics 5.4.0

[scttcper]

i agree. I'd like to see a start/go/enable() function of some kind on each of them.

[rossedfort]

@scttcper could I take a stab at implementing it for the Google Analytics provider? Or would you prefer they be done all at once?

[scttcper]

can just do one if you want, but i wouldn't put it in master until all of them and the docs show how to use it

any contribution is more than i've been doing in the last month or 2 and welcome

[rossedfort]

I've created PR #283 for this Issue, could a maintainer please take a look?

[scttcper]

hey i'm waiting to hear back from one of the other maintainers about updating our npm token thats blocking us from publishing right now. i don't have rights on npm.

i'd like to add another breaking change or to as well before getting into master.
going to start up the "next" branch again and merge this in.

[scttcper]

🎉 This issue has been resolved in version 7.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀