From 07766f8019d571943538ad02eae7c0942ab82a69 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 24 Oct 2023 12:01:02 +0100 Subject: [PATCH] chore: type checking in one place makes bundle smaller (#843) * chore: does type checking in one place make bundle smaller? * fix * fix * fix * is null too * custom linters to get started with --- .eslintrc.js | 19 +++ eslint-rules/README.md | 8 ++ eslint-rules/custom-eslint-rules.test.js | 124 ++++++++++++++++++ eslint-rules/index.js | 17 +++ eslint-rules/no-direct-array-check.js | 20 +++ eslint-rules/no-direct-boolean-check.js | 38 ++++++ eslint-rules/no-direct-date-check.js | 32 +++++ eslint-rules/no-direct-function-check.js | 52 ++++++++ eslint-rules/no-direct-null-check.js | 21 +++ eslint-rules/no-direct-number-check.js | 38 ++++++ eslint-rules/no-direct-object-check.js | 29 ++++ eslint-rules/no-direct-string-check.js | 36 +++++ eslint-rules/no-direct-undefined-check.js | 20 +++ package.json | 11 +- src/autocapture-utils.ts | 12 +- src/autocapture.ts | 15 ++- src/extensions/exceptions/error-conversion.ts | 5 +- src/extensions/exceptions/type-checking.ts | 6 +- src/extensions/sessionrecording-utils.ts | 5 +- src/extensions/sessionrecording.ts | 6 +- src/loader-recorder-v2.ts | 5 +- src/loader-recorder.ts | 3 +- src/loader-surveys.ts | 3 +- src/posthog-core.ts | 22 ++-- src/posthog-persistence.ts | 8 +- src/retry-queue.ts | 4 +- src/send-request.ts | 4 +- src/sessionid.ts | 4 +- src/storage.ts | 6 +- src/utils.ts | 24 ++-- src/uuidv7.ts | 7 +- yarn.lock | 26 ++++ 32 files changed, 565 insertions(+), 65 deletions(-) create mode 100644 eslint-rules/README.md create mode 100644 eslint-rules/custom-eslint-rules.test.js create mode 100644 eslint-rules/index.js create mode 100644 eslint-rules/no-direct-array-check.js create mode 100644 eslint-rules/no-direct-boolean-check.js create mode 100644 eslint-rules/no-direct-date-check.js create mode 100644 eslint-rules/no-direct-function-check.js create mode 100644 eslint-rules/no-direct-null-check.js create mode 100644 eslint-rules/no-direct-number-check.js create mode 100644 eslint-rules/no-direct-object-check.js create mode 100644 eslint-rules/no-direct-string-check.js create mode 100644 eslint-rules/no-direct-undefined-check.js diff --git a/.eslintrc.js b/.eslintrc.js index 74f9e049e..f84ab25e1 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -19,6 +19,7 @@ const extend = [ 'plugin:react-hooks/recommended', 'prettier', 'plugin:compat/recommended', + 'plugin:posthog-js/all', ] module.exports = { @@ -44,6 +45,12 @@ module.exports = { react: { version: '17.0', }, + 'import/resolver': { + node: { + paths: ['eslint-rules'], // Add the directory containing your custom rules + extensions: ['.js', '.jsx', '.ts', '.tsx'], // Ensure ESLint resolves both JS and TS files + }, + }, }, overrides: [ { @@ -57,6 +64,18 @@ module.exports = { 'no-console': 'off', }, }, + { + files: 'eslint-rules/**/*', + extends: ['eslint:recommended', 'prettier'], + rules: { + 'prettier/prettier': 'error', + '@typescript-eslint/no-var-requires': 'off', + 'posthog-js/no-direct-null-check': 'off', + }, + env: { + node: true, + }, + }, ], root: true, } diff --git a/eslint-rules/README.md b/eslint-rules/README.md new file mode 100644 index 000000000..3af38e4f2 --- /dev/null +++ b/eslint-rules/README.md @@ -0,0 +1,8 @@ +# PostHog JS Custom ESLint rules + +This package contains custom ESLint rules for PostHog's JS codebase. + +For example, we have a number of functions that check types like `_isNull` or `isBoolean`. +In most projects these don't help very much but since posthog-js is bundled and included in many different projects, +we want to ensure the bundle size is as small as possible. Moving to these functions reduced bundle by 1%, so we +use a set of custom linters to ensure we don't accidentally add new code that does not use these helpers. diff --git a/eslint-rules/custom-eslint-rules.test.js b/eslint-rules/custom-eslint-rules.test.js new file mode 100644 index 000000000..a3737f0cb --- /dev/null +++ b/eslint-rules/custom-eslint-rules.test.js @@ -0,0 +1,124 @@ +const noDirectNullCheck = require('./no-direct-null-check') +const noDirectUndefinedCheck = require('./no-direct-undefined-check') +const noDirectArrayCheck = require('./no-direct-array-check') +const noDirectIsFunctionCheck = require('./no-direct-function-check') +const noDirectObjectCheck = require('./no-direct-object-check') +const noDirectStringCheck = require('./no-direct-string-check') +const noDirectDateCheck = require('./no-direct-date-check') +const noDirectNumberCheck = require('./no-direct-number-check') +const noDirectBooleanCheck = require('./no-direct-boolean-check') + +const { RuleTester } = require('eslint') + +const ruleTester = new RuleTester() + +ruleTester.run('no-direct-null-check', noDirectNullCheck, { + valid: [{ code: `_isNull(x)` }], + invalid: [{ code: `x === null`, errors: [{ message: 'Use _isNull() instead of direct null checks.' }] }], +}) + +ruleTester.run('no-direct-undefined-check', noDirectUndefinedCheck, { + valid: [{ code: `_isUndefined(x)` }], + invalid: [ + { + code: `typeof x === undefined`, + errors: [{ message: 'Use _isUndefined() instead of direct undefined checks.' }], + }, + ], +}) + +ruleTester.run('no-direct-array-check', noDirectArrayCheck, { + valid: [{ code: `_isArray(x)` }], + invalid: [ + { + code: `Array.isArray(x)`, + errors: [{ message: 'Use _isArray() instead of direct array checks.' }], + }, + ], +}) + +ruleTester.run('no-direct-is-function-check', noDirectIsFunctionCheck, { + valid: [{ code: `_isFunction(x)` }], + invalid: [ + { + code: `/^\\s*\\bfunction\\b/.test(x)`, + errors: [{ message: 'Do not use regex to check for functions. Use _isFunction instead.' }], + }, + { + code: `x instanceof Function`, + errors: [{ message: "Do not use 'instanceof Function' to check for functions. Use _isFunction instead." }], + }, + { + code: `typeof x === "function"`, + errors: [ + { message: 'Do not use \'typeof x === "function"\' to check for functions. Use _isFunction instead.' }, + ], + }, + ], +}) + +ruleTester.run('no-direct-object-check', noDirectObjectCheck, { + valid: [{ code: `_isObject(x)` }], + invalid: [ + { + code: `obj === Object(obj)`, + errors: [{ message: "Do not use 'obj === Object(obj)'. Use _isObject instead." }], + }, + ], +}) + +ruleTester.run('no-direct-string-check', noDirectStringCheck, { + valid: [{ code: `_isString(x)` }], + invalid: [ + { + code: `toString.call(x) == '[object String]'`, + errors: [{ message: 'Use _isString instead of direct string checks.' }], + }, + { + code: `x instanceof String`, + errors: [{ message: 'Use _isString instead of direct string checks.' }], + }, + ], +}) + +ruleTester.run('no-direct-date-check', noDirectDateCheck, { + valid: [{ code: `_isDate(x)` }], + invalid: [ + { + code: `toString.call(obj) == '[object Date]'`, + errors: [{ message: 'Use _isDate instead of direct date checks.' }], + }, + { + code: `x instanceof Date`, + errors: [{ message: 'Use _isDate instead of direct date checks.' }], + }, + ], +}) + +ruleTester.run('no-direct-number-check', noDirectNumberCheck, { + valid: [{ code: `_isNumber(x)` }], + invalid: [ + { + code: `toString.call(obj) == '[object Number]'`, + errors: [{ message: 'Use _isNumber instead of direct number checks.' }], + }, + { + code: `typeof x === 'number'`, + errors: [{ message: 'Use _isNumber instead of direct number checks.' }], + }, + ], +}) + +ruleTester.run('no-direct-boolean-check', noDirectBooleanCheck, { + valid: [{ code: `_isBoolean(x)` }], + invalid: [ + { + code: `toString.call(obj) == '[object Boolean]'`, + errors: [{ message: 'Use _isBoolean instead of direct boolean checks.' }], + }, + { + code: `typeof x === 'boolean'`, + errors: [{ message: 'Use _isBoolean instead of direct boolean checks.' }], + }, + ], +}) diff --git a/eslint-rules/index.js b/eslint-rules/index.js new file mode 100644 index 000000000..6bbd5ad37 --- /dev/null +++ b/eslint-rules/index.js @@ -0,0 +1,17 @@ +const { readdirSync } = require('fs') +const { basename } = require('path') + +const projectName = 'posthog-js' +const ruleFiles = readdirSync('eslint-rules').filter( + (file) => file.endsWith('.js') && file !== 'index.js' && !file.endsWith('test.js') +) +const configs = { + all: { + plugins: [projectName], + rules: Object.fromEntries(ruleFiles.map((file) => [`${projectName}/${basename(file, '.js')}`, 'error'])), + }, +} + +const rules = Object.fromEntries(ruleFiles.map((file) => [basename(file, '.js'), require('./' + file)])) + +module.exports = { configs, rules } diff --git a/eslint-rules/no-direct-array-check.js b/eslint-rules/no-direct-array-check.js new file mode 100644 index 000000000..926704360 --- /dev/null +++ b/eslint-rules/no-direct-array-check.js @@ -0,0 +1,20 @@ +module.exports = { + create(context) { + return { + MemberExpression: function (node) { + // Check if the object is 'Array' and the property is 'isArray' + if ( + node.object.type === 'Identifier' && + node.object.name === 'Array' && + node.property.type === 'Identifier' && + node.property.name === 'isArray' + ) { + context.report({ + node, + message: 'Use _isArray() instead of direct array checks.', + }) + } + }, + } + }, +} diff --git a/eslint-rules/no-direct-boolean-check.js b/eslint-rules/no-direct-boolean-check.js new file mode 100644 index 000000000..c05a4f297 --- /dev/null +++ b/eslint-rules/no-direct-boolean-check.js @@ -0,0 +1,38 @@ +module.exports = { + create(context) { + return { + BinaryExpression: function (node) { + // Check for `toString.call(obj) == '[object Boolean]'` + if ( + (node.operator === '==' || node.operator === '===') && + node.left.type === 'CallExpression' && + node.left.callee.property && + node.left.callee.property.name === 'call' && + node.left.callee.object && + node.left.callee.object.name === 'toString' && + node.right.type === 'Literal' && + node.right.value === '[object Boolean]' + ) { + context.report({ + node, + message: 'Use _isBoolean instead of direct boolean checks.', + }) + } + + // Check for `typeof x === 'boolean'` + if ( + node.operator === '===' && + node.left.type === 'UnaryExpression' && + node.left.operator === 'typeof' && + node.right.type === 'Literal' && + node.right.value === 'boolean' + ) { + context.report({ + node, + message: 'Use _isBoolean instead of direct boolean checks.', + }) + } + }, + } + }, +} diff --git a/eslint-rules/no-direct-date-check.js b/eslint-rules/no-direct-date-check.js new file mode 100644 index 000000000..4be885aa6 --- /dev/null +++ b/eslint-rules/no-direct-date-check.js @@ -0,0 +1,32 @@ +module.exports = { + create(context) { + return { + BinaryExpression: function (node) { + // Check for `toString.call(obj) == '[object Date]'` + if ( + (node.operator === '==' || node.operator === '===') && + node.left.type === 'CallExpression' && + node.left.callee.property && + node.left.callee.property.name === 'call' && + node.left.callee.object && + node.left.callee.object.name === 'toString' && + node.right.type === 'Literal' && + node.right.value === '[object Date]' + ) { + context.report({ + node, + message: 'Use _isDate instead of direct date checks.', + }) + } + + // Check for `x instanceof Date` + if (node.operator === 'instanceof' && node.right.type === 'Identifier' && node.right.name === 'Date') { + context.report({ + node, + message: 'Use _isDate instead of direct date checks.', + }) + } + }, + } + }, +} diff --git a/eslint-rules/no-direct-function-check.js b/eslint-rules/no-direct-function-check.js new file mode 100644 index 000000000..313099ce2 --- /dev/null +++ b/eslint-rules/no-direct-function-check.js @@ -0,0 +1,52 @@ +module.exports = { + create(context) { + return { + CallExpression: function (node) { + // Check if the callee object is a regex matching function pattern + const isFunctionPatternRegex = + node.callee.type === 'MemberExpression' && + node.callee.object?.type === 'Literal' && + node.callee.object?.regex && + node.callee.object?.regex.pattern === '^\\s*\\bfunction\\b' + + // Check if the callee property is 'test' + const isTestCall = node.callee.property?.type === 'Identifier' && node.callee.property?.name === 'test' + + if (isFunctionPatternRegex && isTestCall) { + context.report({ + node, + message: 'Do not use regex to check for functions. Use _isFunction instead.', + }) + } + }, + BinaryExpression: function (node) { + // Check if the operator is 'instanceof' and the right operand is 'Function' + if ( + node.operator === 'instanceof' && + node.right.type === 'Identifier' && + node.right.name === 'Function' + ) { + context.report({ + node, + message: "Do not use 'instanceof Function' to check for functions. Use _isFunction instead.", + }) + } + + // Check for 'typeof x === "function"' pattern + if ( + node.operator === '===' && + node.left.type === 'UnaryExpression' && + node.left.operator === 'typeof' && + node.right.type === 'Literal' && + node.right.value === 'function' + ) { + context.report({ + node, + message: + 'Do not use \'typeof x === "function"\' to check for functions. Use _isFunction instead.', + }) + } + }, + } + }, +} diff --git a/eslint-rules/no-direct-null-check.js b/eslint-rules/no-direct-null-check.js new file mode 100644 index 000000000..49c1d5d78 --- /dev/null +++ b/eslint-rules/no-direct-null-check.js @@ -0,0 +1,21 @@ +module.exports = { + create(context) { + return { + BinaryExpression: function (node) { + // Check if either the left or right operand is a null literal + if ( + (node.left.type === 'Literal' && node.left.value === null) || + (node.right.type === 'Literal' && node.right.value === null) + ) { + // Check the operator to ensure it's a comparison (you can expand this list if needed) + if (node.operator === '===' || node.operator === '!==') { + context.report({ + node, + message: 'Use _isNull() instead of direct null checks.', + }) + } + } + }, + } + }, +} diff --git a/eslint-rules/no-direct-number-check.js b/eslint-rules/no-direct-number-check.js new file mode 100644 index 000000000..7cc2dd2dc --- /dev/null +++ b/eslint-rules/no-direct-number-check.js @@ -0,0 +1,38 @@ +module.exports = { + create(context) { + return { + BinaryExpression: function (node) { + // Check for `toString.call(obj) == '[object Number]'` + if ( + (node.operator === '==' || node.operator === '===') && + node.left.type === 'CallExpression' && + node.left.callee.property && + node.left.callee.property.name === 'call' && + node.left.callee.object && + node.left.callee.object.name === 'toString' && + node.right.type === 'Literal' && + node.right.value === '[object Number]' + ) { + context.report({ + node, + message: 'Use _isNumber instead of direct number checks.', + }) + } + + // Check for `typeof x === 'number'` + if ( + node.operator === '===' && + node.left.type === 'UnaryExpression' && + node.left.operator === 'typeof' && + node.right.type === 'Literal' && + node.right.value === 'number' + ) { + context.report({ + node, + message: 'Use _isNumber instead of direct number checks.', + }) + } + }, + } + }, +} diff --git a/eslint-rules/no-direct-object-check.js b/eslint-rules/no-direct-object-check.js new file mode 100644 index 000000000..e727d9b1a --- /dev/null +++ b/eslint-rules/no-direct-object-check.js @@ -0,0 +1,29 @@ +module.exports = { + create(context) { + return { + BinaryExpression: function (node) { + // Check if the operator is '===' + if (node.operator === '===') { + // Check if left operand is an Identifier (e.g., 'obj') + const isLeftIdentifier = node.left.type === 'Identifier' + + // Check if right operand is a CallExpression with the Object constructor + const isRightObjectCall = + node.right.type === 'CallExpression' && + node.right.callee.type === 'Identifier' && + node.right.callee.name === 'Object' && + node.right.arguments.length === 1 && + node.right.arguments[0].type === 'Identifier' && + node.right.arguments[0].name === node.left.name + + if (isLeftIdentifier && isRightObjectCall) { + context.report({ + node, + message: "Do not use 'obj === Object(obj)'. Use _isObject instead.", + }) + } + } + }, + } + }, +} diff --git a/eslint-rules/no-direct-string-check.js b/eslint-rules/no-direct-string-check.js new file mode 100644 index 000000000..2ae5f66cd --- /dev/null +++ b/eslint-rules/no-direct-string-check.js @@ -0,0 +1,36 @@ +module.exports = { + create(context) { + return { + BinaryExpression: function (node) { + // Check for `toString.call(x) == '[object String]'` + if ( + (node.operator === '==' || node.operator === '===') && + node.left.type === 'CallExpression' && + node.left.callee.property && + node.left.callee.property.name === 'call' && + node.left.callee.object && + node.left.callee.object.name === 'toString' && + node.right.type === 'Literal' && + node.right.value === '[object String]' + ) { + context.report({ + node, + message: 'Use _isString instead of direct string checks.', + }) + } + + // Check for `x instanceof String` + if ( + node.operator === 'instanceof' && + node.right.type === 'Identifier' && + node.right.name === 'String' + ) { + context.report({ + node, + message: 'Use _isString instead of direct string checks.', + }) + } + }, + } + }, +} diff --git a/eslint-rules/no-direct-undefined-check.js b/eslint-rules/no-direct-undefined-check.js new file mode 100644 index 000000000..eca7f2c3a --- /dev/null +++ b/eslint-rules/no-direct-undefined-check.js @@ -0,0 +1,20 @@ +module.exports = { + create(context) { + return { + BinaryExpression: function (node) { + if ( + (node.left.type === 'Identifier' && node.left.name === 'undefined') || + (node.right.type === 'Identifier' && node.right.name === 'undefined') + ) { + // Check the operator to ensure it's a comparison (you can expand this list if needed) + if (node.operator === '===' || node.operator === '!==') { + context.report({ + node, + message: 'Use _isUndefined() instead of direct undefined checks.', + }) + } + } + }, + } + }, +} diff --git a/package.json b/package.json index f3661a4d4..ec9cfcedd 100644 --- a/package.json +++ b/package.json @@ -12,9 +12,11 @@ "build-react": "cd react; yarn; yarn build;", "lint": "eslint src", "prettier": "prettier --write src/ functional_tests/", - "prepublishOnly": "yarn lint && yarn test && yarn build && yarn test-react", - "test": "jest src", - "test-react": "cd react; yarn test", + "prepublishOnly": "yarn lint && yarn test && yarn build && yarn test:react", + "test": "yarn test:unit && yarn test:custom-eslint-rules", + "test:unit": "jest src", + "test:custom-eslint-rules": "jest eslint-rules", + "test:react": "cd react; yarn test", "test-watch": "jest --watch src", "cypress": "cypress open", "prepare": "husky install" @@ -32,6 +34,8 @@ "fflate": "^0.4.1" }, "devDependencies": { + "eslint-config-posthog-js": "link:./eslint-rules", + "eslint-plugin-posthog-js": "link:./eslint-rules", "@babel/core": "7.18.9", "@babel/preset-env": "7.18.9", "@babel/preset-typescript": "^7.18.6", @@ -43,6 +47,7 @@ "@rrweb/types": "^2.0.0-alpha.11", "@sentry/types": "7.37.2", "@testing-library/dom": "^9.3.0", + "@types/eslint": "^8.44.6", "@types/jest": "^29.5.1", "@types/react-dom": "^18.0.10", "@types/uuid": "^9.0.1", diff --git a/src/autocapture-utils.ts b/src/autocapture-utils.ts index a1267bd0b..c0d3dc97f 100644 --- a/src/autocapture-utils.ts +++ b/src/autocapture-utils.ts @@ -4,7 +4,7 @@ * @returns {string} the element's class */ import { AutocaptureConfig } from 'types' -import { _each, _includes, _isUndefined, _trim, logger } from './utils' +import { _each, _includes, _isNull, _isString, _isUndefined, _trim, logger } from './utils' export function getClassName(el: Element): string { switch (typeof el.className) { @@ -210,7 +210,7 @@ export function shouldCaptureElement(el: Element): boolean { // don't include hidden or password fields const type = (el as HTMLInputElement).type || '' - if (typeof type === 'string') { + if (_isString(type)) { // it's possible for el.type to be a DOM element if el is a form with a child input[name="type"] switch (type.toLowerCase()) { case 'hidden': @@ -225,7 +225,7 @@ export function shouldCaptureElement(el: Element): boolean { // See https://github.com/posthog/posthog-js/issues/165 // Under specific circumstances a bug caused .replace to be called on a DOM element // instead of a string, removing the element from the page. Ensure this issue is mitigated. - if (typeof name === 'string') { + if (_isString(name)) { // it's possible for el.name or el.id to be a DOM element if el is a form with a child input[name="name"] const sensitiveNameRegex = /^cc|cardnum|ccnum|creditcard|csc|cvc|cvv|exp|pass|pwd|routing|seccode|securitycode|securitynum|socialsec|socsec|ssn/i @@ -264,11 +264,11 @@ export function isSensitiveElement(el: Element): boolean { * @returns {boolean} whether the element should be captured */ export function shouldCaptureValue(value: string): boolean { - if (value === null || _isUndefined(value)) { + if (_isNull(value) || _isUndefined(value)) { return false } - if (typeof value === 'string') { + if (_isString(value)) { value = _trim(value) // check to see if input value looks like a credit card number @@ -297,7 +297,7 @@ export function shouldCaptureValue(value: string): boolean { * @returns {boolean} whether the element is an angular tag */ export function isAngularStyleAttr(attributeName: string): boolean { - if (typeof attributeName === 'string') { + if (_isString(attributeName)) { return attributeName.substring(0, 10) === '_ngcontent' || attributeName.substring(0, 7) === '_nghost' } return false diff --git a/src/autocapture.ts b/src/autocapture.ts index 3feb9b70f..a96bff61a 100644 --- a/src/autocapture.ts +++ b/src/autocapture.ts @@ -3,7 +3,9 @@ import { _each, _extend, _includes, + _isBoolean, _isFunction, + _isNull, _isUndefined, _register_event, _safewrap_instance_methods, @@ -42,10 +44,9 @@ const autocapture = { _isAutocaptureEnabled: false as boolean, _setIsAutocaptureEnabled: function (instance: PostHog): void { - const disabled_server_side = - this._isDisabledServerSide === null - ? !!instance.persistence?.props[AUTOCAPTURE_DISABLED_SERVER_SIDE] - : this._isDisabledServerSide + const disabled_server_side = _isNull(this._isDisabledServerSide) + ? !!instance.persistence?.props[AUTOCAPTURE_DISABLED_SERVER_SIDE] + : this._isDisabledServerSide const enabled_client_side = !!instance.config.autocapture this._isAutocaptureEnabled = enabled_client_side && !disabled_server_side }, @@ -173,7 +174,7 @@ const autocapture = { _getEventTarget: function (e: Event): Element | null { // https://developer.mozilla.org/en-US/docs/Web/API/Event/target#Compatibility_notes - if (typeof e.target === 'undefined') { + if (_isUndefined(e.target)) { return (e.srcElement as Element) || null } else { if ((e.target as HTMLElement)?.shadowRoot) { @@ -296,7 +297,7 @@ const autocapture = { config: undefined as AutocaptureConfig | undefined, init: function (instance: PostHog): void { - if (typeof instance.__autocapture !== 'boolean') { + if (!_isBoolean(instance.__autocapture)) { this.config = instance.__autocapture } @@ -345,7 +346,7 @@ const autocapture = { // this is a mechanism to ramp up CE with no server-side interaction. // when CE is active, every page load results in a decide request. we - // need to gently ramp this up so we don't overload decide. this decides + // need to gently ramp this up, so we don't overload decide. this decides // deterministically if CE is enabled for this project by modding the char // value of the project token. enabledForProject: function ( diff --git a/src/extensions/exceptions/error-conversion.ts b/src/extensions/exceptions/error-conversion.ts index dbb95c8f9..276f1d41e 100644 --- a/src/extensions/exceptions/error-conversion.ts +++ b/src/extensions/exceptions/error-conversion.ts @@ -9,6 +9,7 @@ import { isPrimitive, } from './type-checking' import { defaultStackParser, StackFrame } from './stack-trace' +import { _isNumber, _isString, _isUndefined } from '../../utils' /** * based on the very wonderful MIT licensed Sentry SDK @@ -42,7 +43,7 @@ const reactMinifiedRegexp = /Minified React error #\d+;/i function getPopSize(ex: Error & { framesToPop?: number }): number { if (ex) { - if (typeof ex.framesToPop === 'number') { + if (_isNumber(ex.framesToPop)) { return ex.framesToPop } @@ -129,7 +130,7 @@ export function errorToProperties([event, source, lineno, colno, error]: ErrorEv $exception_message?: string } = {} - if (error === undefined && typeof event === 'string') { + if (_isUndefined(error) && _isString(event)) { let name = 'Error' let message = event const groups = event.match(ERROR_TYPES_PATTERN) diff --git a/src/extensions/exceptions/type-checking.ts b/src/extensions/exceptions/type-checking.ts index f6ca3b5b1..e8f6f5abe 100644 --- a/src/extensions/exceptions/type-checking.ts +++ b/src/extensions/exceptions/type-checking.ts @@ -1,5 +1,7 @@ +import { _isFunction, _isNull, _isObject, _isUndefined } from '../../utils' + export function isEvent(candidate: unknown): candidate is Event { - return typeof Event !== 'undefined' && isInstanceOf(candidate, Event) + return !_isUndefined(Event) && isInstanceOf(candidate, Event) } export function isPlainObject(candidate: unknown): candidate is Record { @@ -17,7 +19,7 @@ export function isInstanceOf(candidate: unknown, base: any): boolean { export function isPrimitive( candidate: unknown ): candidate is number | string | boolean | bigint | symbol | null | undefined { - return candidate === null || (typeof candidate !== 'object' && typeof candidate !== 'function') + return _isNull(candidate) || (!_isObject(candidate) && !_isFunction(candidate)) } export function isError(candidate: unknown): candidate is Error { diff --git a/src/extensions/sessionrecording-utils.ts b/src/extensions/sessionrecording-utils.ts index 64378c751..0e2719720 100644 --- a/src/extensions/sessionrecording-utils.ts +++ b/src/extensions/sessionrecording-utils.ts @@ -11,6 +11,7 @@ import type { mutationCallbackParam, } from '@rrweb/types' import type { Mirror, MaskInputOptions, MaskInputFn, MaskTextFn, SlimDOMOptions, DataURLOptions } from 'rrweb-snapshot' +import { _isObject } from '../utils' export const replacementImageURI = 'data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMTYiIGhlaWdodD0iMTYiIHZpZXdCb3g9IjAgMCAxNiAxNiIgZmlsbD0ibm9uZSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj4KPHJlY3Qgd2lkdGg9IjE2IiBoZWlnaHQ9IjE2IiBmaWxsPSJibGFjayIvPgo8cGF0aCBkPSJNOCAwSDE2TDAgMTZWOEw4IDBaIiBmaWxsPSIjMkQyRDJEIi8+CjxwYXRoIGQ9Ik0xNiA4VjE2SDhMMTYgOFoiIGZpbGw9IiMyRDJEMkQiLz4KPC9zdmc+Cg==' @@ -106,9 +107,9 @@ export function truncateLargeConsoleLogs(_event: eventWithTime) { if ( event && - typeof event === 'object' && + _isObject(event) && event.type === PLUGIN_EVENT_TYPE && - typeof event.data === 'object' && + _isObject(event.data) && event.data.plugin === CONSOLE_LOG_PLUGIN_NAME ) { // Note: event.data.payload.payload comes from rr-web, and is an array of strings diff --git a/src/extensions/sessionrecording.ts b/src/extensions/sessionrecording.ts index 1efc5ac36..74afc5fa6 100644 --- a/src/extensions/sessionrecording.ts +++ b/src/extensions/sessionrecording.ts @@ -17,7 +17,7 @@ import { PostHog } from '../posthog-core' import { DecideResponse, NetworkRequest, Properties } from '../types' import { EventType, type eventWithTime, type listenerHandler } from '@rrweb/types' import Config from '../config' -import { logger, loadScript, _timestamp, window } from '../utils' +import { logger, loadScript, _timestamp, window, _isUndefined, _isObject } from '../utils' const BASE_ENDPOINT = '/s/' @@ -206,7 +206,7 @@ export class SessionRecording { if (!sessionManager) { return } - if (typeof Object.assign === 'undefined') { + if (_isUndefined(Object.assign)) { // According to the rrweb docs, rrweb is not supported on IE11 and below: // "rrweb does not support IE11 and below because it uses the MutationObserver API which was supported by these browsers." // https://github.com/rrweb-io/rrweb/blob/master/guide.md#compatibility-note @@ -400,7 +400,7 @@ export class SessionRecording { } onRRwebEmit(rawEvent: eventWithTime) { - if (!rawEvent || typeof rawEvent !== 'object') { + if (!rawEvent || !_isObject(rawEvent)) { return } diff --git a/src/loader-recorder-v2.ts b/src/loader-recorder-v2.ts index 66a243879..7d6fcd6ea 100644 --- a/src/loader-recorder-v2.ts +++ b/src/loader-recorder-v2.ts @@ -7,10 +7,9 @@ import rrwebRecord from 'rrweb/es/rrweb/packages/rrweb/src/record' // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore import { getRecordConsolePlugin } from 'rrweb/es/rrweb/packages/rrweb/src/plugins/console/record' -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore +import { _isUndefined } from './utils' -const win: Window & typeof globalThis = typeof window !== 'undefined' ? window : ({} as typeof window) +const win: Window & typeof globalThis = _isUndefined(window) ? ({} as typeof window) : window ;(win as any).rrweb = { record: rrwebRecord, version: 'v2', rrwebVersion: version } ;(win as any).rrwebConsoleRecord = { getRecordConsolePlugin } diff --git a/src/loader-recorder.ts b/src/loader-recorder.ts index 20d1ff696..3531a54c8 100644 --- a/src/loader-recorder.ts +++ b/src/loader-recorder.ts @@ -7,10 +7,11 @@ import rrwebRecord from 'rrweb-v1/es/rrweb/packages/rrweb/src/record' // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore import { getRecordConsolePlugin } from 'rrweb-v1/es/rrweb/packages/rrweb/src/plugins/console/record' +import { _isUndefined } from './utils' // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore -const win: Window & typeof globalThis = typeof window !== 'undefined' ? window : ({} as typeof window) +const win: Window & typeof globalThis = _isUndefined(window) ? ({} as typeof window) : window ;(win as any).rrweb = { record: rrwebRecord, version: 'v1', rrwebVersion: version } ;(win as any).rrwebConsoleRecord = { getRecordConsolePlugin } diff --git a/src/loader-surveys.ts b/src/loader-surveys.ts index 489ddefb0..e0a0886eb 100644 --- a/src/loader-surveys.ts +++ b/src/loader-surveys.ts @@ -1,6 +1,7 @@ import { generateSurveys } from './extensions/surveys' +import { _isUndefined } from './utils' -const win: Window & typeof globalThis = typeof window !== 'undefined' ? window : ({} as typeof window) +const win: Window & typeof globalThis = _isUndefined(window) ? ({} as typeof window) : window ;(win as any).extendPostHogWithSurveys = generateSurveys diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 5f5fa12d0..a6bfd6990 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -17,6 +17,8 @@ import { window, logger, isCrossDomainCookie, + _isString, + _isFunction, } from './utils' import { autocapture } from './autocapture' import { PostHogFeatureFlags } from './posthog-featureflags' @@ -247,7 +249,7 @@ const create_phlib = function ( // if target is not defined, we called init after the lib already // loaded, so there won't be an array of things to execute - if (typeof target !== 'undefined' && _isArray(target)) { + if (!_isUndefined(target) && _isArray(target)) { // Crunch through the people queue first - we queue this data up & // flush on identify, so it's better to do all these operations first instance._execute_array.call(instance.people, (target as any).people) @@ -323,12 +325,12 @@ export class PostHog { // NOTE: See the property definition for deprecation notice this.people = { set: (prop: string | Properties, to?: string, callback?: RequestCallback) => { - const setProps = typeof prop === 'string' ? { [prop]: to } : prop + const setProps = _isString(prop) ? { [prop]: to } : prop this.setPersonProperties(setProps) callback?.({}) }, set_once: (prop: string | Properties, to?: string, callback?: RequestCallback) => { - const setProps = typeof prop === 'string' ? { [prop]: to } : prop + const setProps = _isString(prop) ? { [prop]: to } : prop this.setPersonProperties(undefined, setProps) callback?.({}) }, @@ -731,15 +733,11 @@ export class PostHog { fn_name = item[0] if (_isArray(fn_name)) { capturing_calls.push(item) // chained call e.g. posthog.get_group().set() - } else if (typeof item === 'function') { + } else if (_isFunction(item)) { ;(item as any).call(this) } else if (_isArray(item) && fn_name === 'alias') { alias_calls.push(item) - } else if ( - _isArray(item) && - fn_name.indexOf('capture') !== -1 && - typeof (this as any)[fn_name] === 'function' - ) { + } else if (_isArray(item) && fn_name.indexOf('capture') !== -1 && _isFunction((this as any)[fn_name])) { capturing_calls.push(item) } else { other_calls.push(item) @@ -852,7 +850,7 @@ export class PostHog { } // typing doesn't prevent interesting data - if (_isUndefined(event_name) || typeof event_name !== 'string') { + if (_isUndefined(event_name) || !_isString(event_name)) { logger.error('No event name provided to posthog.capture') return } @@ -966,7 +964,7 @@ export class PostHog { } // set $duration if time_event was previously called for this event - if (typeof start_timestamp !== 'undefined') { + if (!_isUndefined(start_timestamp)) { const duration_in_ms = new Date().getTime() - start_timestamp properties['$duration'] = parseFloat((duration_in_ms / 1000).toFixed(3)) } @@ -1705,7 +1703,7 @@ export class PostHog { Config.DEBUG = true } - if (this.sessionRecording && typeof config.disable_session_recording !== 'undefined') { + if (this.sessionRecording && !_isUndefined(config.disable_session_recording)) { if (oldConfig.disable_session_recording !== config.disable_session_recording) { if (config.disable_session_recording) { this.sessionRecording.stopRecording() diff --git a/src/posthog-persistence.ts b/src/posthog-persistence.ts index 5514e1a03..319e9851c 100644 --- a/src/posthog-persistence.ts +++ b/src/posthog-persistence.ts @@ -88,7 +88,7 @@ export class PostHogPersistence { const p: Properties = {} // Filter out reserved properties _each(this.props, function (v, k) { - if (k === ENABLED_FEATURE_FLAGS && typeof v === 'object') { + if (k === ENABLED_FEATURE_FLAGS && _isObject(v)) { const keys = Object.keys(v) for (let i = 0; i < keys.length; i++) { p[`$feature/${keys[i]}`] = v[keys[i]] @@ -146,10 +146,10 @@ export class PostHogPersistence { register_once(props: Properties, default_value: any, days?: number): boolean { if (_isObject(props)) { - if (typeof default_value === 'undefined') { + if (_isUndefined(default_value)) { default_value = 'None' } - this.expire_days = typeof days === 'undefined' ? this.default_expiry : days + this.expire_days = _isUndefined(days) ? this.default_expiry : days let hasChanges = false @@ -175,7 +175,7 @@ export class PostHogPersistence { register(props: Properties, days?: number): boolean { if (_isObject(props)) { - this.expire_days = typeof days === 'undefined' ? this.default_expiry : days + this.expire_days = _isUndefined(days) ? this.default_expiry : days let hasChanges = false diff --git a/src/retry-queue.ts b/src/retry-queue.ts index 8f799aaf8..d4bfea4ae 100644 --- a/src/retry-queue.ts +++ b/src/retry-queue.ts @@ -1,7 +1,7 @@ import { RequestQueueScaffold } from './base-request-queue' import { encodePostData, xhr } from './send-request' import { QueuedRequestData, RetryQueueElement } from './types' -import { logger } from './utils' +import { _isUndefined, logger } from './utils' import { RateLimiter } from './rate-limiter' const thirtyMinutes = 30 * 60 * 1000 @@ -40,7 +40,7 @@ export class RetryQueue extends RequestQueueScaffold { this.onXHRError = onXHRError this.rateLimiter = rateLimiter - if (typeof window !== 'undefined' && 'onLine' in window.navigator) { + if (!_isUndefined(window) && 'onLine' in window.navigator) { this.areWeOnline = window.navigator.onLine window.addEventListener('online', () => { this._handleWeAreNowOnline() diff --git a/src/send-request.ts b/src/send-request.ts index 7ad121693..1c70deb22 100644 --- a/src/send-request.ts +++ b/src/send-request.ts @@ -1,4 +1,4 @@ -import { _each, _HTTPBuildQuery, logger } from './utils' +import { _each, _HTTPBuildQuery, _isFunction, logger } from './utils' import Config from './config' import { PostData, XHROptions, XHRParams } from './types' @@ -102,7 +102,7 @@ export const xhr = ({ callback(response) } } else { - if (typeof onXHRError === 'function') { + if (_isFunction(onXHRError)) { onXHRError(req) } diff --git a/src/sessionid.ts b/src/sessionid.ts index 2f4ae5203..170acac09 100644 --- a/src/sessionid.ts +++ b/src/sessionid.ts @@ -3,7 +3,7 @@ import { SESSION_ID } from './constants' import { sessionStore } from './storage' import { PostHogConfig, SessionIdChangedCallback } from './types' import { uuidv7 } from './uuidv7' -import { logger } from './utils' +import { _isNumber, logger } from './utils' const MAX_SESSION_IDLE_TIMEOUT = 30 * 60 // 30 mins const MIN_SESSION_IDLE_TIMEOUT = 60 // 1 mins @@ -32,7 +32,7 @@ export class SessionIdManager { const persistenceName = config['persistence_name'] || config['token'] let desiredTimeout = config['session_idle_timeout_seconds'] || MAX_SESSION_IDLE_TIMEOUT - if (typeof desiredTimeout !== 'number') { + if (!_isNumber(desiredTimeout)) { logger.warn('session_idle_timeout_seconds must be a number. Defaulting to 30 minutes.') desiredTimeout = MAX_SESSION_IDLE_TIMEOUT } else if (desiredTimeout > MAX_SESSION_IDLE_TIMEOUT) { diff --git a/src/storage.ts b/src/storage.ts index dc0665e08..02f14bbc2 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -1,4 +1,4 @@ -import { _extend, logger } from './utils' +import { _extend, _isUndefined, logger } from './utils' import { PersistentStore, Properties } from './types' import { DISTINCT_ID, SESSION_ID } from './constants' @@ -96,7 +96,7 @@ export const localStore: PersistentStore = { } let supported = true - if (typeof window !== 'undefined') { + if (!_isUndefined(window)) { try { const key = '__mplssupport__', val = 'xyz' @@ -249,7 +249,7 @@ export const sessionStore: PersistentStore = { return sessionStorageSupported } sessionStorageSupported = true - if (typeof window !== 'undefined') { + if (!_isUndefined(window)) { try { const key = '__support__', val = 'xyz' diff --git a/src/utils.ts b/src/utils.ts index eb3b26176..7d765f124 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -67,7 +67,7 @@ export const _trim = function (str: string): string { export const _bind_instance_methods = function (obj: Record): void { for (const func in obj) { - if (typeof obj[func] === 'function') { + if (_isFunction(obj[func])) { obj[func] = obj[func].bind(obj) } } @@ -97,10 +97,10 @@ export function _eachArray( * @param {Object=} thisArg */ export function _each(obj: any, iterator: (value: any, key: any) => void | Breaker, thisArg?: any): void { - if (obj === null || obj === undefined) { + if (_isNull(obj) || _isUndefined(obj)) { return } - if (Array.isArray(obj)) { + if (_isArray(obj)) { return _eachArray(obj, iterator, thisArg) } for (const key in obj) { @@ -145,7 +145,7 @@ export const _include = function ( target: any ): boolean | Breaker { let found = false - if (obj === null) { + if (_isNull(obj)) { return found } if (nativeIndexOf && obj.indexOf === nativeIndexOf) { @@ -200,6 +200,10 @@ export const _isUndefined = function (obj: any): obj is undefined { return obj === void 0 } +export const _isNull = function (obj: any): obj is null { + return obj === null +} + export const _isString = function (obj: any): obj is string { return toString.call(obj) == '[object String]' } @@ -212,6 +216,10 @@ export const _isNumber = function (obj: any): obj is number { return toString.call(obj) == '[object Number]' } +export const _isBoolean = function (obj: any): obj is boolean { + return toString.call(obj) == '[object Boolean]' +} + export const _isValidRegex = function (str: string): boolean { try { new RegExp(str) @@ -288,7 +296,7 @@ export const _safewrap_class = function (klass: Function, functions: string[]): export const _safewrap_instance_methods = function (obj: Record): void { for (const func in obj) { - if (typeof obj[func] === 'function') { + if (_isFunction(obj[func])) { obj[func] = _safewrap(obj[func]) } } @@ -354,7 +362,7 @@ export function _copyAndTruncateStrings = Record -1) { return value } - if (typeof value === 'string' && maxStringLength !== null) { + if (_isString(value) && maxStringLength !== null) { return (value as string).slice(0, maxStringLength) } return value @@ -541,7 +549,7 @@ export const _getQueryParam = function (url: string, param: string): string { const regexS = '[\\?&]' + cleanParam + '=([^&#]*)' const regex = new RegExp(regexS) const results = regex.exec(url) - if (results === null || (results && typeof results[1] !== 'string' && (results[1] as any).length)) { + if (_isNull(results) || (results && !_isString(results[1]) && (results[1] as any).length)) { return '' } else { let result = results[1] @@ -808,7 +816,7 @@ export const _info = { Mozilla: /rv:(\d+(\.\d+)?)/, } const regex: RegExp | undefined = versionRegexs[browser as keyof typeof versionRegexs] - if (regex === undefined) { + if (_isUndefined(regex)) { return null } const matches = userAgent.match(regex) diff --git a/src/uuidv7.ts b/src/uuidv7.ts index ac5fce06b..aaac5d528 100644 --- a/src/uuidv7.ts +++ b/src/uuidv7.ts @@ -9,6 +9,8 @@ */ // polyfill for IE11 +import { _isNumber, _isUndefined, window } from './utils' + if (!Math.trunc) { Math.trunc = function (v) { return v < 0 ? Math.ceil(v) : Math.floor(v) @@ -18,7 +20,7 @@ if (!Math.trunc) { // polyfill for IE11 if (!Number.isInteger) { Number.isInteger = function (value) { - return typeof value === 'number' && isFinite(value) && Math.floor(value) === value + return _isNumber(value) && isFinite(value) && Math.floor(value) === value } } @@ -203,6 +205,7 @@ declare const UUIDV7_DENY_WEAK_RNG: boolean /** Stores `crypto.getRandomValues()` available in the environment. */ let getRandomValues: (buffer: T) => T = (buffer) => { // fall back on Math.random() unless the flag is set to true + // TRICKY: don't use the _isUndefined method here as can't pass the reference if (typeof UUIDV7_DENY_WEAK_RNG !== 'undefined' && UUIDV7_DENY_WEAK_RNG) { throw new Error('no cryptographically strong RNG available') } @@ -214,7 +217,7 @@ let getRandomValues: (buffer: T) => T = (buf } // detect Web Crypto API -if (typeof crypto !== 'undefined' && crypto.getRandomValues) { +if (!_isUndefined(window.crypto) && crypto.getRandomValues) { getRandomValues = (buffer) => crypto.getRandomValues(buffer) } diff --git a/yarn.lock b/yarn.lock index c9121291d..b7aadba4f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3614,6 +3614,19 @@ resolved "https://registry.yarnpkg.com/@types/error-stack-parser/-/error-stack-parser-1.3.18.tgz#e01c9f8c85ca83b610320c62258b0c9026ade0f7" integrity sha1-4ByfjIXKg7YQMgxiJYsMkCat4Pc= +"@types/eslint@^8.44.6": + version "8.44.6" + resolved "https://registry.yarnpkg.com/@types/eslint/-/eslint-8.44.6.tgz#60e564551966dd255f4c01c459f0b4fb87068603" + integrity sha512-P6bY56TVmX8y9J87jHNgQh43h6VVU+6H7oN7hgvivV81K2XY8qJZ5vqPy/HdUoVIelii2kChYVzQanlswPWVFw== + dependencies: + "@types/estree" "*" + "@types/json-schema" "*" + +"@types/estree@*": + version "1.0.3" + resolved "https://registry.yarnpkg.com/@types/estree/-/estree-1.0.3.tgz#2be19e759a3dd18c79f9f436bd7363556c1a73dd" + integrity sha512-CS2rOaoQ/eAgAfcTfq6amKG7bsN+EMcgGY4FAFQdvSj2y1ixvOZTUA9mOtCai7E1SYu283XNw7urKK30nP3wkQ== + "@types/estree@0.0.39": version "0.0.39" resolved "https://registry.yarnpkg.com/@types/estree/-/estree-0.0.39.tgz#e177e699ee1b8c22d23174caaa7422644389509f" @@ -3671,6 +3684,11 @@ resolved "https://registry.yarnpkg.com/@types/js-levenshtein/-/js-levenshtein-1.1.1.tgz#ba05426a43f9e4e30b631941e0aa17bf0c890ed5" integrity sha512-qC4bCqYGy1y/NP7dDVr7KJarn+PbX1nSpwA7JXdu0HxT3QYjO8MJ+cntENtHFVy2dRAyBV23OZ6MxsW1AM1L8g== +"@types/json-schema@*": + version "7.0.14" + resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.14.tgz#74a97a5573980802f32c8e47b663530ab3b6b7d1" + integrity sha512-U3PUjAudAdJBeC2pgN8uTIKgxrb4nlDF3SF0++EldXQvQBGkpFZMSnwQiIoDU77tv45VgNkl/L4ouD+rEomujw== + "@types/json-schema@^7.0.12": version "7.0.12" resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.12.tgz#d70faba7039d5fca54c83c7dbab41051d2b6f6cb" @@ -5822,6 +5840,10 @@ escodegen@^2.0.0: optionalDependencies: source-map "~0.6.1" +"eslint-config-posthog-js@link:./eslint-rules": + version "0.0.0" + uid "" + eslint-config-prettier@^8.5.0: version "8.5.0" resolved "https://registry.yarnpkg.com/eslint-config-prettier/-/eslint-config-prettier-8.5.0.tgz#5a81680ec934beca02c7b1a61cf8ca34b66feab1" @@ -5848,6 +5870,10 @@ eslint-plugin-jest@^27.2.3: dependencies: "@typescript-eslint/utils" "^5.10.0" +"eslint-plugin-posthog-js@link:./eslint-rules": + version "0.0.0" + uid "" + eslint-plugin-prettier@^4.2.1: version "4.2.1" resolved "https://registry.yarnpkg.com/eslint-plugin-prettier/-/eslint-plugin-prettier-4.2.1.tgz#651cbb88b1dab98bfd42f017a12fa6b2d993f94b"