bmish
commented
2 years ago This is related to the ember/no-side-effects rule.
My company has a custom rule for disallowing event tracking inside computer properties that also sounds related. Haven't open sourced it because it checks for a specific internal event tracking function call, but I'm copying the rule here for reference.
# no-tracking-in-computed-properties
Computed properties should not have side effects (see [ember/no-side-effects])
because it makes it harder to reason about. In particular, tracking events fired in computed properties
may fire more often than desired. Tracking should instead be done in lifecycle hooks, actions, or
or any other imperative code.
## Rule Details
This rule disallows any use of `this.eventstream.track()` in computed properties.
## Examples
Examples of **incorrect** code for this rule:
import Component from '@ember/component';
import { computed } from '@ember/object';
import { inject as service } from '@ember/service';
Component.extend({
eventstream: service(),
someProp: computed(function () {
this.eventstream.track(123);
return 123;
}),
});
## Migration
There is no auto-fixer. These should be manually fixed on a case-by-case basis. Each case might be
fixed in a different way, depending on where the code exists and other constraints.
The example below shows one way of fixing this, if a component is tracking whether a feature is seen.
### Before
import Component from '@ember/component';
import { computed } from '@ember/object';
import { inject as service } from '@ember/service';
Component.extend({
eventstream: service(),
/**
* In this example, assume this property is gating whether some part of the UI shows,
* depending on the value of a feature flag.
*
* The tracking event is fired to indicate that the merchant saw the new feature.
*/
shouldShowNewFeature: computed(
'currentUser.canDashboardShowNewFeature',
function () {
const shouldShowNewFeature = this.currentUser.canDashboardShowNewFeature;
if (shouldShowNewFeature) {
// Linter would error here
this.eventstream.track('This merchant saw the new feature');
}
return shouldShowNewFeature;
}
),
});
### After
Move the tracking event to the component's `init` hook. Depending on the real-life use case, it could perhaps move to the route or another component lifecycle hook. In this example we're moving it to `init`.
import Component from '@ember/component';
import { readOnly } from '@ember/object/computed';
Component.extend({
// Now the computed property has no side effects, and is "pure"
// This makes the computed property easier to reason about, since it is just a value holder.
shouldShowNewFeature: readOnly('currentUser.canDashboardShowNewFeature'),
init(...args) {
this._super(...args);
// The tracking now happens on init.
// It could potentially move to the router, or another component lifecycle hook.
// This gives us more guarantees about when this tracking event will fire.
if (this.shouldShowNewFeature) {
this.eventstream.track('This merchant saw the new feature');
}
},
});
## Related Rules
* [ember/no-side-effects](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-side-effects.md)('use strict');
const { isComputedProp } = require('eslint-plugin-ember/lib/utils/ember');
const { getImportIdentifier } = require('eslint-plugin-ember/lib/utils/import');
const ERROR_MESSAGE = 'Do not track events in computed properties';
function isEventstreamTrackCall(node) {
return (
node.type === 'CallExpression' &&
node.callee.type === 'MemberExpression' &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === 'track' && // the function being called is named "track"
node.callee.object.type === 'MemberExpression' &&
node.callee.object.object.type === 'ThisExpression' && // the line starts with "this."
node.callee.object.property.type === 'Identifier' &&
node.callee.object.property.name === 'eventstream'
); // specifically, "this.eventstream"
}
function isInsideComputedProp(node, importedEmberName, importedComputedName) {
if (node.parent) {
return (
isComputedProp(node.parent, importedEmberName, importedComputedName) ||
isInsideComputedProp(node.parent, importedEmberName, importedComputedName)
);
} else {
return false;
}
}
module.exports = {
ERROR_MESSAGE,
meta: {
type: 'problem',
docs: {
description: 'disallow tracking events in computed properties',
category: 'Ember',
},
schema: [],
},
create(context) {
let importedEmberName;
let importedComputedName;
return {
ImportDeclaration(node) {
if (node.source.value === 'ember') {
importedEmberName =
importedEmberName || getImportIdentifier(node, 'ember');
}
if (node.source.value === '@ember/object') {
importedComputedName =
importedComputedName ||
getImportIdentifier(node, '@ember/object', 'computed');
}
},
CallExpression(node) {
if (
isEventstreamTrackCall(node) &&
isInsideComputedProp(node, importedEmberName, importedComputedName)
) {
context.report({ node, message: ERROR_MESSAGE });
}
},
};
},
};
const RuleTester = require('eslint').RuleTester;
const rule = require('../../../lib/rules/no-tracking-in-computed-properties');
const { ERROR_MESSAGE } = rule;
const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 2021,
sourceType: 'module',
},
});
ruleTester.run('no-tracking-in-computed-properties', rule, {
valid: [
// Standalone calls are allowed
'this.eventstream.track()',
// Calls that are not inside computed properties are allowed
`
function someFunction() {
this.eventstream.track();
}
`,
// Similar calls not on the eventstream service are allowed
`
computed(function() {
this.track();
});
`,
// Calls to other eventstream functions are allowed
`
computed(function() {
this.eventstream.someOtherFunction();
});
`,
// Calls to another service's track function are allowed
`
computed(function() {
this.someOtherService.track();
});
`,
].map(addImports),
invalid: [
{
code: `
computed(function() {
this.eventstream.track(123);
});
`,
output: null,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
{
// Deeply nested calls are disallowed
code: `
computed(function() {
[].forEach(function() {
this.eventstream.track(123);
});
});
`,
output: null,
errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }],
},
].map(addImports),
});
function addImports(objOrCode) {
const imports = "import { computed } from '@ember/object';";
if (typeof objOrCode === 'object') {
objOrCode.code = `${imports} ${objOrCode.code}`; // eslint-disable-line no-param-reassign
return objOrCode;
} else {
return `${imports} '@ember/object'; ${objOrCode}`;
}
}
``
should disallow: