Skip to content

Commit b07afeb

Browse files
[RUMF-848] Expose internal types and constants to be used in Datadog app (#727)
* [RUMF-848] expose internal types and constants to be used in Datadog app * [RUMF-848] add a ESLint rule to enforce declarative modules * [RUMF-848] allow using 'import type' from any file in restricted files * 🚚 [RUMF-848] move types and constants to respect the new ESLint rule * [RUMF-848] adjust ESLint rule * Add some documentation * Enforce it on all code that should be added to the packages * Reword a few functions * Make sure only files with side effects can import files with side effects * Allow `new WeakMap()` and `Object.keys()` calls * 🚚 [RUMF-848] remove tracekit IIFE by spliting it into different modules * [RUMF-848] replace tracekit report.* with proper functions * [RUMF-848] replace tracekit computeStackTrace.* with proper functions * [RUMF-848] move LogsUserConfiguration in logs.ts * [RUMF-848] fix last RRWeb issues * Replace unnecessary RegExp constructor with a regexp value * Disable the rule on 'mutationBuffer', as I want to remove this in the future. * Remove unused `freezePage` function * 👌 [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 ce55e19 commit b07afeb

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+1549
-1312
lines changed

.eslintrc.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ module.exports = {
2121
'eslint-plugin-prefer-arrow',
2222
'eslint-plugin-unicorn',
2323
'@typescript-eslint',
24+
'eslint-plugin-local-rules',
2425
],
2526
rules: {
2627
'@typescript-eslint/array-type': ['error', { default: 'array-simple' }],
@@ -217,5 +218,12 @@ module.exports = {
217218
'@typescript-eslint/restrict-template-expressions': 'off',
218219
},
219220
},
221+
{
222+
files: ['packages/*/src/**/*.ts'],
223+
excludedFiles: '*.spec.ts',
224+
rules: {
225+
'local-rules/disallow-side-effects': 'error',
226+
},
227+
},
220228
],
221229
}

LICENSE-3rdparty.csv

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ dev,emoji-name-map,MIT,Copyright 2016-19 Ionică Bizău <bizauionica@gmail.com>
2929
dev,eslint,MIT,Copyright JS Foundation and other contributors
3030
dev,eslint-config-prettier,MIT,Copyright (c) 2017, 2018, 2019, 2020 Simon Lydell and contributors
3131
dev,eslint-plugin-import,MIT,Copyright (c) 2015 Ben Mosher
32-
dev,eslint-plugin-jsdoc,BSD-3-Clause,Copyright (c) 2018, Gajus Kuizinas (http://gajus.com/)
3332
dev,eslint-plugin-jasmine,MIT,Copyright (c) 2021 Tom Vincent
33+
dev,eslint-plugin-jsdoc,BSD-3-Clause,Copyright (c) 2018, Gajus Kuizinas (http://gajus.com/)
34+
dev,eslint-plugin-local-rules,MIT,Copyright (c) 2017 Clayton Watts
3435
dev,eslint-plugin-prefer-arrow,MIT,Copyright (c) 2018 Triston Jones
3536
dev,eslint-plugin-unicorn,MIT,Copyright (c) Sindre Sorhus <sindresorhus@gmail.com> (https://sindresorhus.com)
3637
dev,express,MIT,Copyright 2009-2014 TJ Holowaychuk 2013-2014 Roman Shtylman 2014-2015 Douglas Christopher Wilson

eslint-local-rules.js

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/* eslint-disable unicorn/filename-case */
2+
const path = require('path')
3+
4+
// Declare the local rules used by the Browser SDK
5+
//
6+
// See https://eslint.org/docs/developer-guide/working-with-rules for documentation on how to write
7+
// rules.
8+
//
9+
// You can use https://astexplorer.net/ to explore the parsed data structure of a code snippet.
10+
// Choose '@typescript-eslint/parser' as a parser to have the exact same structure as our ESLint
11+
// parser.
12+
module.exports = {
13+
'disallow-side-effects': {
14+
meta: {
15+
docs: {
16+
description:
17+
'Disallow potential side effects when evaluating modules, to ensure modules content are tree-shakable.',
18+
recommended: false,
19+
},
20+
schema: [],
21+
},
22+
create(context) {
23+
const filename = context.getFilename()
24+
if (pathsWithSideEffect.has(filename)) {
25+
return {}
26+
}
27+
return {
28+
Program(node) {
29+
reportPotentialSideEffect(context, node)
30+
},
31+
}
32+
},
33+
},
34+
}
35+
36+
const packagesRoot = `${__dirname}/packages`
37+
38+
// Those modules are known to have side effects when evaluated
39+
const pathsWithSideEffect = new Set([
40+
`${packagesRoot}/logs/src/boot/logs.entry.ts`,
41+
`${packagesRoot}/logs/src/index.ts`,
42+
`${packagesRoot}/rum-recorder/src/boot/recorder.entry.ts`,
43+
`${packagesRoot}/rum-recorder/src/index.ts`,
44+
`${packagesRoot}/rum/src/boot/rum.entry.ts`,
45+
`${packagesRoot}/rum/src/index.ts`,
46+
])
47+
48+
// Those packages are known to have no side effects when evaluated
49+
const packagesWithoutSideEffect = new Set(['@datadog/browser-core', '@datadog/browser-rum-core'])
50+
51+
/**
52+
* Iterate over the given node and its children, and report any node that may have a side effect
53+
* when evaluated.
54+
*
55+
* @example
56+
* const foo = 1 // OK, this statement can't have any side effect
57+
* foo() // KO, we don't know what 'foo' does, report this
58+
* function bar() { // OK, a function declaration doesn't have side effects
59+
* foo() // OK, this statement won't be executed when evaluating the module code
60+
* }
61+
*/
62+
function reportPotentialSideEffect(context, node) {
63+
// This acts like an authorized list of syntax nodes to use directly in the body of a module. All
64+
// those nodes should not have a side effect when evaluated.
65+
//
66+
// This list is probably not complete, feel free to add more cases if you encounter an unhandled
67+
// node.
68+
switch (node.type) {
69+
case 'Program':
70+
node.body.forEach((child) => reportPotentialSideEffect(context, child))
71+
return
72+
case 'TemplateLiteral':
73+
node.expressions.forEach((child) => reportPotentialSideEffect(context, child))
74+
return
75+
case 'ExportNamedDeclaration':
76+
case 'ExportAllDeclaration':
77+
case 'ImportDeclaration':
78+
if (node.declaration) {
79+
reportPotentialSideEffect(context, node.declaration)
80+
} else if (
81+
node.source &&
82+
node.importKind !== 'type' &&
83+
!isAllowedImport(context.getFilename(), node.source.value)
84+
) {
85+
context.report({
86+
node: node.source,
87+
message: 'This file cannot import modules with side-effects',
88+
})
89+
}
90+
return
91+
case 'VariableDeclaration':
92+
node.declarations.forEach((child) => reportPotentialSideEffect(context, child))
93+
return
94+
case 'VariableDeclarator':
95+
if (node.init) {
96+
reportPotentialSideEffect(context, node.init)
97+
}
98+
return
99+
case 'ArrayExpression':
100+
node.elements.forEach((child) => reportPotentialSideEffect(context, child))
101+
return
102+
case 'UnaryExpression':
103+
reportPotentialSideEffect(context, node.argument)
104+
return
105+
case 'ObjectExpression':
106+
node.properties.forEach((child) => reportPotentialSideEffect(context, child))
107+
return
108+
case 'SpreadElement':
109+
reportPotentialSideEffect(context, node.argument)
110+
return
111+
case 'Property':
112+
reportPotentialSideEffect(context, node.key)
113+
reportPotentialSideEffect(context, node.value)
114+
return
115+
case 'BinaryExpression':
116+
reportPotentialSideEffect(context, node.left)
117+
reportPotentialSideEffect(context, node.right)
118+
return
119+
case 'TSAsExpression':
120+
case 'ExpressionStatement':
121+
reportPotentialSideEffect(context, node.expression)
122+
return
123+
case 'MemberExpression':
124+
reportPotentialSideEffect(context, node.object)
125+
reportPotentialSideEffect(context, node.property)
126+
return
127+
case 'FunctionExpression':
128+
case 'ArrowFunctionExpression':
129+
case 'FunctionDeclaration':
130+
case 'ClassDeclaration':
131+
case 'TSEnumDeclaration':
132+
case 'TSInterfaceDeclaration':
133+
case 'TSTypeAliasDeclaration':
134+
case 'TSDeclareFunction':
135+
case 'Literal':
136+
case 'Identifier':
137+
return
138+
case 'CallExpression':
139+
if (isAllowedCallExpression(node)) {
140+
return
141+
}
142+
case 'NewExpression':
143+
if (isAllowedNewExpression(node)) {
144+
return
145+
}
146+
}
147+
148+
// If the node doesn't match any of the condition above, report it
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+
})
154+
}
155+
156+
/**
157+
* Make sure an 'import' statement does not pull a module or package with side effects.
158+
*/
159+
function isAllowedImport(basePath, source) {
160+
if (source.startsWith('.')) {
161+
const resolvedPath = `${path.resolve(path.dirname(basePath), source)}.ts`
162+
return !pathsWithSideEffect.has(resolvedPath)
163+
}
164+
return packagesWithoutSideEffect.has(source)
165+
}
166+
167+
/* eslint-disable max-len */
168+
/**
169+
* Authorize some call expressions. Feel free to add more exceptions here. Good candidates would
170+
* be functions that are known to be ECMAScript functions without side effects, that are likely to
171+
* be considered as pure functions by the bundler.
172+
*
173+
* You can experiment with Rollup tree-shaking strategy to ensure your function is known to be pure.
174+
* https://rollupjs.org/repl/?version=2.38.5&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMiUyRiUyRiUyMFB1cmUlMjBmdW5jdGlvbnMlNUNubmV3JTIwV2Vha01hcCgpJTVDbk9iamVjdC5rZXlzKCklNUNuJTVDbiUyRiUyRiUyMFNpZGUlMjBlZmZlY3QlMjBmdW5jdGlvbnMlNUNuZm9vKCklMjAlMkYlMkYlMjB1bmtub3duJTIwZnVuY3Rpb25zJTIwYXJlJTIwY29uc2lkZXJlZCUyMHRvJTIwaGF2ZSUyMHNpZGUlMjBlZmZlY3RzJTVDbmFsZXJ0KCdhYWEnKSU1Q25uZXclMjBNdXRhdGlvbk9ic2VydmVyKCgpJTIwJTNEJTNFJTIwJTdCJTdEKSUyMiUyQyUyMmlzRW50cnklMjIlM0F0cnVlJTdEJTVEJTJDJTIyb3B0aW9ucyUyMiUzQSU3QiUyMmZvcm1hdCUyMiUzQSUyMmVzJTIyJTJDJTIybmFtZSUyMiUzQSUyMm15QnVuZGxlJTIyJTJDJTIyYW1kJTIyJTNBJTdCJTIyaWQlMjIlM0ElMjIlMjIlN0QlMkMlMjJnbG9iYWxzJTIyJTNBJTdCJTdEJTdEJTJDJTIyZXhhbXBsZSUyMiUzQW51bGwlN0Q=
175+
*
176+
* Webpack is not as smart as Rollup, and it usually treat all call expressions as impure, but it
177+
* could be fine to allow it nonetheless at it pulls very little code.
178+
*/
179+
/* eslint-enable max-len */
180+
function isAllowedCallExpression({ callee }) {
181+
// Allow "Object.keys()"
182+
if (callee.type === 'MemberExpression' && callee.object.name === 'Object' && callee.property.name === 'keys') {
183+
return true
184+
}
185+
186+
return false
187+
}
188+
189+
/* eslint-disable max-len */
190+
/**
191+
* Authorize some 'new' expressions. Feel free to add more exceptions here. Good candidates would
192+
* be functions that are known to be ECMAScript functions without side effects, that are likely to
193+
* be considered as pure functions by the bundler.
194+
*
195+
* You can experiment with Rollup tree-shaking strategy to ensure your function is known to be pure.
196+
* https://rollupjs.org/repl/?version=2.38.5&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMiUyRiUyRiUyMFB1cmUlMjBmdW5jdGlvbnMlNUNubmV3JTIwV2Vha01hcCgpJTVDbk9iamVjdC5rZXlzKCklNUNuJTVDbiUyRiUyRiUyMFNpZGUlMjBlZmZlY3QlMjBmdW5jdGlvbnMlNUNuZm9vKCklMjAlMkYlMkYlMjB1bmtub3duJTIwZnVuY3Rpb25zJTIwYXJlJTIwY29uc2lkZXJlZCUyMHRvJTIwaGF2ZSUyMHNpZGUlMjBlZmZlY3RzJTVDbmFsZXJ0KCdhYWEnKSU1Q25uZXclMjBNdXRhdGlvbk9ic2VydmVyKCgpJTIwJTNEJTNFJTIwJTdCJTdEKSUyMiUyQyUyMmlzRW50cnklMjIlM0F0cnVlJTdEJTVEJTJDJTIyb3B0aW9ucyUyMiUzQSU3QiUyMmZvcm1hdCUyMiUzQSUyMmVzJTIyJTJDJTIybmFtZSUyMiUzQSUyMm15QnVuZGxlJTIyJTJDJTIyYW1kJTIyJTNBJTdCJTIyaWQlMjIlM0ElMjIlMjIlN0QlMkMlMjJnbG9iYWxzJTIyJTNBJTdCJTdEJTdEJTJDJTIyZXhhbXBsZSUyMiUzQW51bGwlN0Q=
197+
*
198+
* Webpack is not as smart as Rollup, and it usually treat all 'new' expressions as impure, but it
199+
* could be fine to allow it nonetheless at it pulls very little code.
200+
*/
201+
/* eslint-enable max-len */
202+
function isAllowedNewExpression({ callee }) {
203+
// Allow "new WeakMap()"
204+
if (callee.name === 'WeakMap') {
205+
return true
206+
}
207+
208+
return false
209+
}

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
"eslint-plugin-import": "2.22.1",
5454
"eslint-plugin-jasmine": "4.1.2",
5555
"eslint-plugin-jsdoc": "30.7.13",
56+
"eslint-plugin-local-rules": "1.0.1",
5657
"eslint-plugin-prefer-arrow": "1.2.2",
5758
"eslint-plugin-unicorn": "25.0.1",
5859
"express": "4.17.1",

packages/core/src/domain/automaticErrorCollection.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Observable } from '../tools/observable'
55
import { jsonStringify, ONE_MINUTE, RequestType } from '../tools/utils'
66
import { Configuration } from './configuration'
77
import { monitor } from './internalMonitoring'
8-
import { computeStackTrace, Handler, report, StackTrace } from './tracekit'
8+
import { computeStackTrace, subscribe, unsubscribe, StackTrace } from './tracekit'
99

1010
export type ErrorObservable = Observable<RawError>
1111
let filteredErrorsObservable: ErrorObservable
@@ -82,11 +82,11 @@ export function startRuntimeErrorTracking(errorObservable: ErrorObservable) {
8282
startTime: performance.now(),
8383
})
8484
}
85-
;(report.subscribe as (handler: Handler) => void)(traceKitReportHandler)
85+
subscribe(traceKitReportHandler)
8686
}
8787

8888
export function stopRuntimeErrorTracking() {
89-
;(report.unsubscribe as (handler: Handler) => void)(traceKitReportHandler)
89+
unsubscribe(traceKitReportHandler)
9090
}
9191

9292
export function trackNetworkError(configuration: Configuration, errorObservable: ErrorObservable) {

0 commit comments

Comments
 (0)