Analytics error fallback, fix opt-out #495
Conversation
There was a problem hiding this comment.
Thanks for the finding and the PR, this one was really bugging me 👍
I have made a few comments regarding the form, but otherwise the idea for the workaround is perfect.
Also before commit, you should run npm run lint:fix to auto-fix code style issues, you can check that everything is good after with npm run lint so that the CI will be happy 😺
generators/app/index.js
Outdated
| try{ | ||
| this.insight = (!this.options.analytics || process.env.DISABLE_NGX_ANALYTICS) ? { optOut: true } : new Insight({trackingCode: 'UA-93069862-1', pkg}); | ||
| }catch{ | ||
| this.insight = { optOut: true }; |
There was a problem hiding this comment.
Would be better to just noop the track function then, ie: this.insight = { track: () => {} };
generators/app/index.js
Outdated
| @@ -68,7 +73,9 @@ class NgxGenerator extends Generator { | |||
| )})\n` | |||
| ); | |||
| this.log(`${chalk.yellow("Make sure you don't have uncommitted changes before overwriting files!")}`); | |||
| this.insight.track('update', fromVersion, 'to', this.version); | |||
| if(!this.insight.optOut){ | |||
There was a problem hiding this comment.
no need for all this (and following checks) using if insight.track is noop'ed 😉
generators/app/index.js
Outdated
| 'tools-hads': props => props.tools && props.tools.includes('hads'), | ||
| 'tools-jest': props => props.tools && props.tools.includes('jest'), | ||
| 'tools-karma': props => props.tools && !props.tools.includes('jest') | ||
| 'tools-hads': props => props.tools && props.tools.includes('hads') |
There was a problem hiding this comment.
whoops, make sure you rebase your code on the current master or this will cause a regression
|
I think the |
|
I've learned that I know even less about git through the CLI than I thought... I think I've got everything figured out though, opening another pull request here in a sec with the comments fixed. Thanks for the info, learned some new things about JS today (I still suck at JS, but hey, learning is fun lol) |
Fixed the analytics opt-out functionality to check if the user has opted out before using Insight library for analytics. Checks if something in the Insight library failed (i.e. on initialization) and if so, trip the no-analytics flag.