Skip to content

Commit

Permalink
Prevent usage of node or browser APIs in @segment/analytics-core and …
Browse files Browse the repository at this point in the history
…@segment/analytics-node (#846)
  • Loading branch information
silesky authored Apr 24, 2023
1 parent 151a58f commit 7dcafa2
Show file tree
Hide file tree
Showing 17 changed files with 433 additions and 83 deletions.
5 changes: 5 additions & 0 deletions .changeset/giant-gifts-jump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@segment/analytics-core': patch
---

Remove browser-specific isOffline() logic from core
25 changes: 25 additions & 0 deletions .eslintrc.isomorphic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/** @type { import('eslint').Linter.Config } */
module.exports = {
extends: ['./.eslintrc'],
plugins: ['import'],
overrides: [
{
// this library should not have any node OR browser runtime dependencies.
// In particular, nextjs and vercel edge functions will break if _any_ node dependencies are introduced.
files: ['src/**'],
excludedFiles: ['**/__tests__/**'],
rules: {
'no-restricted-globals': [
'error',
'document',
'window',
'self',
'process',
'global',
'navigator',
],
'import/no-nodejs-modules': 'error',
},
},
],
}
26 changes: 13 additions & 13 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,13 @@ module.exports = {
parserOptions: {
ecmaVersion: 2020,
},
env: {
es6: true,
node: true,
browser: true,
},
extends: [
// Turn on on eslint recommended rules https://github.com/eslint/eslint/blob/main/conf/eslint-recommended.js
'eslint:recommended',
// Turn off eslint rules that conflict with prettier https://github.com/prettier/eslint-config-prettier/blob/main/index.js
'prettier',
],
overrides: [
{
files: ['*.{js,mjs}'],
extends: [
// Handle prettier rules through eslint https://github.com/prettier/eslint-plugin-prettier/blob/master/eslint-plugin-prettier.js#L65
'plugin:prettier/recommended',
],
},
{
files: ['*.{ts,tsx}'],
parser: '@typescript-eslint/parser',
Expand Down Expand Up @@ -65,13 +53,25 @@ module.exports = {
},
overrides: [
{
files: ['*.test.*'],
files: ['**/__tests__/**', '**/scripts/**'],
rules: {
'require-await': 'off',
'@typescript-eslint/require-await': 'off',
},
},
],
},
{
files: ['*.{js,mjs}'],
env: {
// Config files and possible scripts. Allow anything, we don't really care.
browser: true,
node: true,
},
extends: [
// Handle prettier rules through eslint https://github.com/prettier/eslint-plugin-prettier/blob/master/eslint-plugin-prettier.js#L65
'plugin:prettier/recommended',
],
},
],
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"concurrently": "^7.6.0",
"eslint": "^8.14.0",
"eslint-config-prettier": "^8.5.0",
"eslint-plugin-import": "^2.27.5",
"eslint-plugin-prettier": "^4.0.0",
"express": "^4.18.2",
"get-monorepo-packages": "^1.2.0",
Expand Down
5 changes: 5 additions & 0 deletions packages/browser/src/core/queue/event-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@ import { PersistedPriorityQueue } from '../../lib/priority-queue/persisted'
import { Context } from '../context'
import { AnyBrowserPlugin } from '../plugin'
import { CoreEventQueue } from '@segment/analytics-core'
import { isOffline } from '../connection'

export class EventQueue extends CoreEventQueue<Context, AnyBrowserPlugin> {
constructor(priorityQueue?: PriorityQueue<Context>) {
super(priorityQueue ?? new PersistedPriorityQueue(4, 'event-queue'))
}
async flush(): Promise<Context[]> {
if (isOffline()) return []
return super.flush()
}
}
2 changes: 1 addition & 1 deletion packages/core/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @type { import('eslint').Linter.Config } */
module.exports = {
extends: ['../../.eslintrc'],
extends: ['../../.eslintrc.isomorphic'],
}
11 changes: 0 additions & 11 deletions packages/core/src/analytics/__tests__/dispatch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,6 @@ type JestMockedFn<Fn> = Fn extends (...args: infer Args) => infer ReturnT
? jest.Mock<ReturnT, Args>
: never

const isOnline = jest.fn().mockReturnValue(true)
const isOffline = jest.fn().mockReturnValue(false)

jest.mock('../../connection', () => ({
isOnline,
isOffline,
}))

const fetcher: JestMockedFn<typeof import('node-fetch')['default']> = jest.fn()
jest.mock('node-fetch', () => fetcher)

const invokeCallback: JestMockedFn<
typeof import('../../callback')['invokeCallback']
> = jest.fn()
Expand Down
32 changes: 0 additions & 32 deletions packages/core/src/connection/__tests__/index.test.ts

This file was deleted.

13 changes: 0 additions & 13 deletions packages/core/src/connection/index.ts

This file was deleted.

3 changes: 1 addition & 2 deletions packages/core/src/queue/event-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { Integrations, JSONObject } from '../events/interfaces'
import { CorePlugin } from '../plugins'
import { createTaskGroup, TaskGroup } from '../task/task-group'
import { attempt, ensure } from './delivery'
import { isOffline } from '../connection'

export type EventQueueEmitterContract<Ctx extends CoreContext> = {
message_delivered: [ctx: Ctx]
Expand Down Expand Up @@ -190,7 +189,7 @@ export abstract class CoreEventQueue<
}

async flush(): Promise<Ctx[]> {
if (this.queue.length === 0 || isOffline()) {
if (this.queue.length === 0) {
return []
}

Expand Down
7 changes: 0 additions & 7 deletions packages/core/src/utils/environment.ts

This file was deleted.

1 change: 1 addition & 0 deletions packages/core/src/utils/get-global.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-restricted-globals */
// This an imperfect polyfill for globalThis
export const getGlobal = () => {
if (typeof globalThis !== 'undefined') {
Expand Down
5 changes: 1 addition & 4 deletions packages/node/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
/** @type { import('eslint').Linter.Config } */
module.exports = {
extends: ['../../.eslintrc'],
env: {
node: true,
},
extends: ['../../.eslintrc.isomorphic'],
rules: {
'@typescript-eslint/no-empty-interface': 'off', // since this is a lib, sometimes we want to use interfaces rather than types for the ease of declaration merging.
},
Expand Down
1 change: 1 addition & 0 deletions packages/node/src/lib/base-64-encode.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// eslint-disable-next-line import/no-nodejs-modules
import { Buffer } from 'buffer'
/**
* Base64 encoder that works in browser, worker, node runtimes.
Expand Down
1 change: 1 addition & 0 deletions packages/node/src/lib/env.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-restricted-globals */
export type RuntimeEnv =
| 'node'
| 'browser'
Expand Down
1 change: 1 addition & 0 deletions packages/node/src/plugins/segmentio/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ function normalizeEvent(ctx: Context) {
ctx.updateEvent('context.library.version', version)
const runtime = detectRuntime()
if (runtime === 'node') {
// eslint-disable-next-line no-restricted-globals
ctx.updateEvent('_metadata.nodeVersion', process.version)
}
ctx.updateEvent('_metadata.jsRuntime', runtime)
Expand Down
Loading

0 comments on commit 7dcafa2

Please sign in to comment.