Skip to content

Commit 44d2b84

Browse files
👌 [RUMF-848] review adjustments
* rename the rule to `disallow-side-effects` * adjust doc comments * remove unneeded eslint-disable comment * improve rule description and report message
1 parent 01f2da1 commit 44d2b84

File tree

4 files changed

+13
-11
lines changed

4 files changed

+13
-11
lines changed

.eslintrc.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ module.exports = {
222222
files: ['packages/*/src/**/*.ts'],
223223
excludedFiles: '*.spec.ts',
224224
rules: {
225-
'local-rules/enforce-declarative-modules': 'error',
225+
'local-rules/disallow-side-effects': 'error',
226226
},
227227
},
228228
],

eslint-local-rules.js

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,11 @@ const path = require('path')
1010
// Choose '@typescript-eslint/parser' as a parser to have the exact same structure as our ESLint
1111
// parser.
1212
module.exports = {
13-
'enforce-declarative-modules': {
13+
'disallow-side-effects': {
1414
meta: {
1515
docs: {
16-
description: 'Disallow potential side effects in when evaluating modules',
16+
description:
17+
'Disallow potential side effects when evaluating modules, to ensure modules content are tree-shakable.',
1718
recommended: false,
1819
},
1920
schema: [],
@@ -59,7 +60,7 @@ const packagesWithoutSideEffect = new Set(['@datadog/browser-core', '@datadog/br
5960
* }
6061
*/
6162
function reportPotentialSideEffect(context, node) {
62-
// This acts like a authorized list of syntax nodes to use directly in the body of a module. All
63+
// This acts like an authorized list of syntax nodes to use directly in the body of a module. All
6364
// those nodes should not have a side effect when evaluated.
6465
//
6566
// This list is probably not complete, feel free to add more cases if you encounter an unhandled
@@ -145,7 +146,11 @@ function reportPotentialSideEffect(context, node) {
145146
}
146147

147148
// If the node doesn't match any of the condition above, report it
148-
context.report({ node, message: `${node.type} not allowed in types, constants and internal files` })
149+
context.report({
150+
node,
151+
message: `${node.type} can have side effects when the module is evaluated. \
152+
Maybe move it in a function declaration?`,
153+
})
149154
}
150155

151156
/**
@@ -161,7 +166,7 @@ function isAllowedImport(basePath, source) {
161166

162167
/* eslint-disable max-len */
163168
/**
164-
* Authorize some call expressions. Feel free to add more exceptions here. Good candidates would
169+
* Authorize some call expressions. Feel free to add more exceptions here. Good candidates would
165170
* be functions that are known to be ECMAScript functions without side effects, that are likely to
166171
* be considered as pure functions by the bundler.
167172
*
@@ -183,7 +188,7 @@ function isAllowedCallExpression({ callee }) {
183188

184189
/* eslint-disable max-len */
185190
/**
186-
* Authorize some 'new' expressions. Feel free to add more exceptions here. Good candidates would
191+
* Authorize some 'new' expressions. Feel free to add more exceptions here. Good candidates would
187192
* be functions that are known to be ECMAScript functions without side effects, that are likely to
188193
* be considered as pure functions by the bundler.
189194
*

packages/rum-core/src/rawRumEvent.types.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
// Ideally, we could use `import type` here, but since we are still supporting TS 3.0, we can't do
2-
// this for now.
3-
// eslint-disable-next-line local-rules/enforce-declarative-modules
41
import { Context, ErrorSource, ResourceType } from '@datadog/browser-core'
52

63
export enum RumEventType {

packages/rum-recorder/src/domain/rrweb/observer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import {
4040
} from './utils'
4141

4242
// TODO: remove this global MutationBuffer instance
43-
// eslint-disable-next-line local-rules/enforce-declarative-modules
43+
// eslint-disable-next-line local-rules/disallow-side-effects
4444
export const mutationBuffer = new MutationBuffer()
4545

4646
function initMutationObserver(

0 commit comments

Comments
 (0)