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,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,
 }

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.

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.' }],
+        },
+    ],
+})

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 }

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.',
+                    })
+                }
+            },
+        }
+    },
+}

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.',
+                    })
+                }
+            },
+        }
+    },
+}

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.',
+                    })
+                }
+            },
+        }
+    },
+}

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.',
+                    })
+                }
+            },
+        }
+    },
+}

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.',
+                        })
+                    }
+                }
+            },
+        }
+    },
+}

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.',
+                    })
+                }
+            },
+        }
+    },
+}