Skip to content

Commit

Permalink
fix: Remove underscores from all functions (#1140)
Browse files Browse the repository at this point in the history
  • Loading branch information
benjackwhite authored Apr 18, 2024
1 parent f8adc00 commit cd0e341
Show file tree
Hide file tree
Showing 59 changed files with 507 additions and 554 deletions.
6 changes: 3 additions & 3 deletions cypress/e2e/session-recording.cy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// <reference types="cypress" />

import { _isNull } from '../../src/utils/type-utils'
import { isNull } from '../../src/utils/type-utils'
import { start } from '../support/setup'
import { assertWhetherPostHogRequestsWereCalled, pollPhCaptures } from '../support/assertions'

Expand Down Expand Up @@ -185,7 +185,7 @@ describe('Session recording', () => {
cy.wait('@session-recording').then(() => {
cy.phCaptures({ full: true }).then((captures) => {
captures.forEach((c) => {
if (_isNull(sessionId)) {
if (isNull(sessionId)) {
sessionId = c.properties['$session_id']
}
// all captures should be from one session
Expand Down Expand Up @@ -248,7 +248,7 @@ describe('Session recording', () => {
cy.wait('@session-recording').then(() => {
cy.phCaptures({ full: true }).then((captures) => {
captures.forEach((c) => {
if (_isNull(sessionId)) {
if (isNull(sessionId)) {
sessionId = c.properties['$session_id']
}
// all captures should be from one session
Expand Down
2 changes: 1 addition & 1 deletion eslint-rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

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`.
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.
48 changes: 24 additions & 24 deletions eslint-rules/custom-eslint-rules.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,112 +13,112 @@ 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.' }] }],
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)` }],
valid: [{ code: `isUndefined(x)` }],
invalid: [
{
code: `typeof x === undefined`,
errors: [{ message: 'Use _isUndefined() instead of direct undefined checks.' }],
errors: [{ message: 'Use isUndefined() instead of direct undefined checks.' }],
},
],
})

ruleTester.run('no-direct-array-check', noDirectArrayCheck, {
valid: [{ code: `_isArray(x)` }],
valid: [{ code: `isArray(x)` }],
invalid: [
{
code: `Array.isArray(x)`,
errors: [{ message: 'Use _isArray() instead of direct array checks.' }],
errors: [{ message: 'Use isArray() instead of direct array checks.' }],
},
],
})

ruleTester.run('no-direct-is-function-check', noDirectIsFunctionCheck, {
valid: [{ code: `_isFunction(x)` }],
valid: [{ code: `isFunction(x)` }],
invalid: [
{
code: `/^\\s*\\bfunction\\b/.test(x)`,
errors: [{ message: 'Do not use regex to check for functions. Use _isFunction instead.' }],
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." }],
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.' },
{ 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)` }],
valid: [{ code: `isObject(x)` }],
invalid: [
{
code: `obj === Object(obj)`,
errors: [{ message: "Do not use 'obj === Object(obj)'. Use _isObject instead." }],
errors: [{ message: "Do not use 'obj === Object(obj)'. Use isObject instead." }],
},
],
})

ruleTester.run('no-direct-string-check', noDirectStringCheck, {
valid: [{ code: `_isString(x)` }],
valid: [{ code: `isString(x)` }],
invalid: [
{
code: `toString.call(x) == '[object String]'`,
errors: [{ message: 'Use _isString instead of direct string checks.' }],
errors: [{ message: 'Use isString instead of direct string checks.' }],
},
{
code: `x instanceof String`,
errors: [{ message: 'Use _isString instead of direct string checks.' }],
errors: [{ message: 'Use isString instead of direct string checks.' }],
},
],
})

ruleTester.run('no-direct-date-check', noDirectDateCheck, {
valid: [{ code: `_isDate(x)` }],
valid: [{ code: `isDate(x)` }],
invalid: [
{
code: `toString.call(obj) == '[object Date]'`,
errors: [{ message: 'Use _isDate instead of direct date checks.' }],
errors: [{ message: 'Use isDate instead of direct date checks.' }],
},
{
code: `x instanceof Date`,
errors: [{ message: 'Use _isDate instead of direct date checks.' }],
errors: [{ message: 'Use isDate instead of direct date checks.' }],
},
],
})

ruleTester.run('no-direct-number-check', noDirectNumberCheck, {
valid: [{ code: `_isNumber(x)` }],
valid: [{ code: `isNumber(x)` }],
invalid: [
{
code: `toString.call(obj) == '[object Number]'`,
errors: [{ message: 'Use _isNumber instead of direct number checks.' }],
errors: [{ message: 'Use isNumber instead of direct number checks.' }],
},
{
code: `typeof x === 'number'`,
errors: [{ message: 'Use _isNumber instead of direct number checks.' }],
errors: [{ message: 'Use isNumber instead of direct number checks.' }],
},
],
})

ruleTester.run('no-direct-boolean-check', noDirectBooleanCheck, {
valid: [{ code: `_isBoolean(x)` }],
valid: [{ code: `isBoolean(x)` }],
invalid: [
{
code: `toString.call(obj) == '[object Boolean]'`,
errors: [{ message: 'Use _isBoolean instead of direct boolean checks.' }],
errors: [{ message: 'Use isBoolean instead of direct boolean checks.' }],
},
{
code: `typeof x === 'boolean'`,
errors: [{ message: 'Use _isBoolean instead of direct boolean checks.' }],
errors: [{ message: 'Use isBoolean instead of direct boolean checks.' }],
},
],
})
2 changes: 1 addition & 1 deletion eslint-rules/no-direct-array-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isArray() instead of direct array checks.',
message: 'Use isArray() instead of direct array checks.',
})
}
},
Expand Down
4 changes: 2 additions & 2 deletions eslint-rules/no-direct-boolean-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isBoolean instead of direct boolean checks.',
message: 'Use isBoolean instead of direct boolean checks.',
})
}

Expand All @@ -29,7 +29,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isBoolean instead of direct boolean checks.',
message: 'Use isBoolean instead of direct boolean checks.',
})
}
},
Expand Down
4 changes: 2 additions & 2 deletions eslint-rules/no-direct-date-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isDate instead of direct date checks.',
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.',
message: 'Use isDate instead of direct date checks.',
})
}
},
Expand Down
4 changes: 2 additions & 2 deletions eslint-rules/no-direct-document-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isDocument instead of direct document checks.',
message: 'Use isDocument instead of direct document checks.',
})
}

Expand All @@ -27,7 +27,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isDocument instead of direct document checks.',
message: 'Use isDocument instead of direct document checks.',
})
}
},
Expand Down
4 changes: 2 additions & 2 deletions eslint-rules/no-direct-file-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isFile instead of direct type checks.',
message: 'Use isFile instead of direct type checks.',
})
}

// Check for `x instanceof File`
if (node.operator === 'instanceof' && node.right.type === 'Identifier' && node.right.name === 'File') {
context.report({
node,
message: 'Use _isFile instead of direct checks.',
message: 'Use isFile instead of direct checks.',
})
}
},
Expand Down
4 changes: 2 additions & 2 deletions eslint-rules/no-direct-form-data-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isFormData instead of direct type checks.',
message: 'Use isFormData instead of direct type checks.',
})
}

Expand All @@ -27,7 +27,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isFormData instead of direct checks.',
message: 'Use isFormData instead of direct checks.',
})
}
},
Expand Down
6 changes: 3 additions & 3 deletions eslint-rules/no-direct-function-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
if (isFunctionPatternRegex && isTestCall) {
context.report({
node,
message: 'Do not use regex to check for functions. Use _isFunction instead.',
message: 'Do not use regex to check for functions. Use isFunction instead.',
})
}
},
Expand All @@ -28,7 +28,7 @@ module.exports = {
) {
context.report({
node,
message: "Do not use 'instanceof Function' to check for functions. Use _isFunction instead.",
message: "Do not use 'instanceof Function' to check for functions. Use isFunction instead.",
})
}

Expand All @@ -43,7 +43,7 @@ module.exports = {
context.report({
node,
message:
'Do not use \'typeof x === "function"\' to check for functions. Use _isFunction instead.',
'Do not use \'typeof x === "function"\' to check for functions. Use isFunction instead.',
})
}
},
Expand Down
2 changes: 1 addition & 1 deletion eslint-rules/no-direct-null-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = {
if (node.operator === '===' || node.operator === '!==') {
context.report({
node,
message: 'Use _isNull() instead of direct null checks.',
message: 'Use isNull() instead of direct null checks.',
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions eslint-rules/no-direct-number-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isNumber instead of direct number checks.',
message: 'Use isNumber instead of direct number checks.',
})
}

Expand All @@ -29,7 +29,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isNumber instead of direct number checks.',
message: 'Use isNumber instead of direct number checks.',
})
}
},
Expand Down
2 changes: 1 addition & 1 deletion eslint-rules/no-direct-object-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = {
if (isLeftIdentifier && isRightObjectCall) {
context.report({
node,
message: "Do not use 'obj === Object(obj)'. Use _isObject instead.",
message: "Do not use 'obj === Object(obj)'. Use isObject instead.",
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions eslint-rules/no-direct-string-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isString instead of direct string checks.',
message: 'Use isString instead of direct string checks.',
})
}

Expand All @@ -27,7 +27,7 @@ module.exports = {
) {
context.report({
node,
message: 'Use _isString instead of direct string checks.',
message: 'Use isString instead of direct string checks.',
})
}
},
Expand Down
2 changes: 1 addition & 1 deletion eslint-rules/no-direct-undefined-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module.exports = {
if (node.operator === '===' || node.operator === '!==') {
context.report({
node,
message: 'Use _isUndefined() instead of direct undefined checks.',
message: 'Use isUndefined() instead of direct undefined checks.',
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
unhandledRejectionToProperties,
} from '../../../extensions/exception-autocapture/error-conversion'

import { _isNull } from '../../../utils/type-utils'
import { isNull } from '../../../utils/type-utils'

// ugh, jest
// can't reference PromiseRejectionEvent to construct it 🤷
Expand Down Expand Up @@ -63,7 +63,7 @@ describe('Error conversion', () => {
const error = new Error('oh no an error has happened')

const errorProperties = errorToProperties(['something', undefined, undefined, undefined, error])
if (_isNull(errorProperties)) {
if (isNull(errorProperties)) {
throw new Error("this mustn't be null")
}

Expand Down Expand Up @@ -93,7 +93,7 @@ describe('Error conversion', () => {
const event = new DOMException('oh no disaster', 'dom-exception')
const errorProperties = errorToProperties([event as unknown as Event])

if (_isNull(errorProperties)) {
if (isNull(errorProperties)) {
throw new Error("this mustn't be null")
}

Expand All @@ -109,7 +109,7 @@ describe('Error conversion', () => {
const event = new ErrorEvent('oh no an error event', { error: new Error('the real error is hidden inside') })

const errorProperties = errorToProperties([event as unknown as Event])
if (_isNull(errorProperties)) {
if (isNull(errorProperties)) {
throw new Error("this mustn't be null")
}

Expand Down
Loading

0 comments on commit cd0e341

Please sign in to comment.