Prevent usage of node or browser APIs in @segment/analytics-core and @segment/analytics-node (#846)

Author: silesky

.changeset/giant-gifts-jump.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-core': patch
+---
+
+Remove browser-specific isOffline() logic from core

.eslintrc.isomorphic.js

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

.eslintrc.js

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

package.json

@@ -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",

packages/browser/src/core/queue/event-queue.ts

@@ -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()
+  }
 }

packages/core/.eslintrc.js

@@ -1,4 +1,4 @@
 /** @type { import('eslint').Linter.Config } */
 module.exports = {
-  extends: ['../../.eslintrc'],
+  extends: ['../../.eslintrc.isomorphic'],
 }

packages/core/src/analytics/__tests__/dispatch.test.ts

@@ -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()

packages/core/src/connection/__tests__/index.test.ts

@@ -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]
@@ -190,7 +189,7 @@ export abstract class CoreEventQueue<
   }
 
   async flush(): Promise<Ctx[]> {
-    if (this.queue.length === 0 || isOffline()) {
+    if (this.queue.length === 0) {
       return []
     }