Make trait nullable and type group vs params

Author: silesky

.buildkite/pipeline.yml

@@ -99,6 +99,7 @@ steps:
       - echo "+++ Run tests"
       - yarn run -T core lint
       - yarn run -T core test
+      - yarn turbo run --filter=core-integration-tests lint
     plugins:
       - ssh://git@github.com/segmentio/cache-buildkite-plugin#v2.0.0:
           key: "v1.1-cache-dev-{{ checksum 'yarn.lock' }}"

.changeset/hip-parents-smell.md

@@ -0,0 +1,7 @@
+---
+'@segment/analytics-core': minor
+'@segment/analytics-next': minor
+'@segment/analytics-node': minor
+---
+
+Make trait fields nullable. Type traits for group() differently than identify() call.

package.json

@@ -23,11 +23,11 @@
     "release": "yarn clean && yarn build --force && changeset publish && git push origin HEAD:master --follow-tags --no-verify",
     "version-run-all": "yarn workspaces foreach -vpt --no-private run version",
     "core": "yarn workspace @segment/analytics-core",
-    "core+deps": "turbo run --filter=@segment/analytics-core...",
+    "core+deps": "turbo run --filter=@segment/analytics-core",
     "browser": "yarn workspace @segment/analytics-next",
-    "browser+deps": "turbo run --filter=@segment/analytics-next...",
+    "browser+deps": "turbo run --filter=@segment/analytics-next",
     "node": "yarn workspace @segment/analytics-node",
-    "node+deps": "turbo run --filter=@segment/analytics-node...",
+    "node+deps": "turbo run --filter=@segment/analytics-node",
     "clean": "bash scripts/clean.sh"
   },
   "packageManager": "yarn@3.2.1",

packages/browser/src/browser/__tests__/typedef-tests/analytics-browser.ts

@@ -119,12 +119,33 @@ export default {
     assertNotAny(analytics)
     assertIs<AnalyticsBrowser>(analytics)
   },
-  'Should error if there is a type conflict in Traits': () => {
-    const analytics = new AnalyticsBrowser().load({ writeKey: 'foo' })
-    assertNotAny(analytics)
-    assertIs<AnalyticsBrowser>(analytics)
 
-    // @ts-expect-error - id should be a string
-    void analytics.identify('foo', { id: 123 })
+  'Should accept traits': () => {
+    const analytics = {} as AnalyticsBrowser
+
+    class Foo {
+      name = 'hello'
+      toJSON() {
+        return this.name
+      }
+    }
+    void analytics.identify('foo', new Foo())
+    void analytics.identify('foo', { address: null })
+    // @ts-expect-error - Type 'string' has no properties in common with type
+    void analytics.identify('foo', { address: 'hello' })
+    // @ts-expect-error - Type 'never[]' is not assignable to type
+    void analytics.identify('foo', { id: [] })
+  },
+
+  'Should accept optional ExtraContext': () => {
+    const analytics = {} as AnalyticsBrowser
+    void analytics.track('foo', undefined, { context: {} })
+    void analytics.track('foo', undefined, { context: { active: true } })
+    void analytics.track('foo', undefined, {
+      context: {
+        // @ts-expect-error Type 'string' is not assignable to type 'boolean | null | undefined'.ts(2322)
+        active: 'hello',
+      },
+    })
   },
 }

packages/browser/src/core/analytics/index.ts

@@ -2,12 +2,13 @@ import {
   AliasParams,
   DispatchedEvent,
   EventParams,
+  GroupParams,
   PageParams,
   resolveAliasArguments,
   resolveArguments,
   resolvePageArguments,
-  resolveUserArguments,
-  UserParams,
+  resolveIdentifyOrGroupArguments,
+  IdentifyParams,
 } from '../arguments-resolver'
 import type { FormArgs, LinkArgs } from '../auto-track'
 import { isOffline } from '../connection'
@@ -202,10 +203,10 @@ export class Analytics
     })
   }
 
-  async identify(...args: UserParams): Promise<DispatchedEvent> {
-    const [id, _traits, options, callback] = resolveUserArguments(this._user)(
-      ...args
-    )
+  async identify(...args: IdentifyParams): Promise<DispatchedEvent> {
+    const [id, _traits, options, callback] = resolveIdentifyOrGroupArguments(
+      this._user
+    )(...args)
 
     this._user.identify(id, _traits)
     const segmentEvent = this.eventFactory.identify(
@@ -227,15 +228,15 @@ export class Analytics
   }
 
   group(): Group
-  group(...args: UserParams): Promise<DispatchedEvent>
-  group(...args: UserParams): Promise<DispatchedEvent> | Group {
+  group(...args: GroupParams): Promise<DispatchedEvent>
+  group(...args: GroupParams): Promise<DispatchedEvent> | Group {
     if (args.length === 0) {
       return this._group
     }
 
-    const [id, _traits, options, callback] = resolveUserArguments(this._group)(
-      ...args
-    )
+    const [id, _traits, options, callback] = resolveIdentifyOrGroupArguments(
+      this._group
+    )(...args)
 
     this._group.identify(id, _traits)
     const groupId = this._group.id()

packages/browser/src/core/analytics/interfaces.ts

@@ -4,8 +4,9 @@ import type {
   EventParams,
   DispatchedEvent,
   PageParams,
-  UserParams,
+  IdentifyParams,
   AliasParams,
+  GroupParams,
 } from '../arguments-resolver'
 import type { Context } from '../context'
 import type { SegmentEvent } from '../events'
@@ -78,9 +79,9 @@ export interface AnalyticsClassic extends AnalyticsClassicStubs {
 export interface AnalyticsCore extends CoreAnalytics {
   track(...args: EventParams): Promise<DispatchedEvent>
   page(...args: PageParams): Promise<DispatchedEvent>
-  identify(...args: UserParams): Promise<DispatchedEvent>
+  identify(...args: IdentifyParams): Promise<DispatchedEvent>
   group(): Group
-  group(...args: UserParams): Promise<DispatchedEvent>
+  group(...args: GroupParams): Promise<DispatchedEvent>
   alias(...args: AliasParams): Promise<DispatchedEvent>
   screen(...args: PageParams): Promise<DispatchedEvent>
   register(...plugins: Plugin[]): Promise<Context>
@@ -94,6 +95,6 @@ export interface AnalyticsCore extends CoreAnalytics {
  */
 export type AnalyticsBrowserCore = Omit<AnalyticsCore, 'group' | 'user'> & {
   group(): Promise<Group>
-  group(...args: UserParams): Promise<DispatchedEvent>
+  group(...args: GroupParams): Promise<DispatchedEvent>
   user(): Promise<User>
 }

packages/browser/src/core/arguments-resolver/__tests__/index.test.ts

@@ -1,7 +1,7 @@
 import {
   resolveArguments,
   resolvePageArguments,
-  resolveUserArguments,
+  resolveIdentifyOrGroupArguments,
   resolveAliasArguments,
 } from '../'
 import { Callback } from '../../events'
@@ -349,9 +349,9 @@ describe(resolvePageArguments, () => {
   })
 })
 
-describe(resolveUserArguments, () => {
+describe(resolveIdentifyOrGroupArguments, () => {
   let user: User
-  let resolver: ReturnType<typeof resolveUserArguments>
+  let resolver: ReturnType<typeof resolveIdentifyOrGroupArguments>
   let fn: Callback
 
   const userTraits = {
@@ -362,7 +362,7 @@ describe(resolveUserArguments, () => {
 
   beforeEach(() => {
     user = new User()
-    resolver = resolveUserArguments(user)
+    resolver = resolveIdentifyOrGroupArguments(user)
     fn = jest.fn()
   })
 

packages/browser/src/core/arguments-resolver/index.ts

@@ -12,8 +12,10 @@ import {
   EventProperties,
   SegmentEvent,
   Traits,
+  GroupTraits,
+  IdentifyTraits,
 } from '../events'
-import { ID, User } from '../user'
+import { Group, ID, User } from '../user'
 
 /**
  * Helper for the track method
@@ -105,8 +107,10 @@ export function resolvePageArguments(
 /**
  * Helper for group, identify methods
  */
-export const resolveUserArguments = (user: User): ResolveUser => {
-  return (...args): ReturnType<ResolveUser> => {
+export const resolveIdentifyOrGroupArguments = <T extends Traits>(
+  user: User | Group
+): ResolveIdentifyOrGroup<T> => {
+  return (...args): ReturnType<ResolveIdentifyOrGroup<T>> => {
     let id: string | ID | null = null
     id = args.find(isString) ?? args.find(isNumber)?.toString() ?? user.id()
 
@@ -117,7 +121,7 @@ export const resolveUserArguments = (user: User): ResolveUser => {
       return isPlainObject(obj) || obj === null
     }) as Array<Traits | null>
 
-    const traits = (objects[0] ?? {}) as Traits
+    const traits = (objects[0] ?? {}) as T
     const opts = (objects[1] ?? {}) as Options
 
     const resolvedCallback = args.find(isFunction) as Callback | undefined
@@ -146,14 +150,15 @@ export function resolveAliasArguments(
   return [aliasTo, aliasFrom, opts, resolvedCallback]
 }
 
-type ResolveUser = (
+type ResolveIdentifyOrGroup<T extends Traits> = (
   id?: ID | object,
-  traits?: Traits | Callback | null,
+  traits?: T | Callback | null,
   options?: Options | Callback,
   callback?: Callback
-) => [ID, Traits, Options, Callback | undefined]
+) => [ID, T, Options, Callback | undefined]
 
-export type UserParams = Parameters<ResolveUser>
+export type IdentifyParams = Parameters<ResolveIdentifyOrGroup<IdentifyTraits>>
+export type GroupParams = Parameters<ResolveIdentifyOrGroup<GroupTraits>>
 export type EventParams = Parameters<typeof resolveArguments>
 export type PageParams = Parameters<typeof resolvePageArguments>
 export type AliasParams = Parameters<typeof resolveAliasArguments>

packages/browser/src/core/events/interfaces.ts

@@ -11,13 +11,28 @@ import {
   JSONValue,
   JSONPrimitive,
   JSONObject,
+  CoreGroupTraits,
+  CoreIndentifyTraits,
 } from '@segment/analytics-core'
 
 export interface Options extends CoreOptions {}
 
-// This is not ideal, but it works with all the edge cases
 export interface Traits extends CoreAnalyticsTraits {}
 
+/**
+ * Traits are pieces of information you know about a group.
+ * This interface represents reserved traits that Segment has standardized.
+ * @link https://segment.com/docs/connections/spec/group/#traits
+ */
+export interface GroupTraits extends CoreGroupTraits {}
+
+/**
+ * Traits are pieces of information you know about a user.
+ * This interface represents reserved traits that Segment has standardized.
+ * @link https://segment.com/docs/connections/spec/identify/#traits
+ */
+export interface IdentifyTraits extends CoreIndentifyTraits {}
+
 export type EventProperties = Record<string, any>
 
 export interface SegmentEvent extends CoreSegmentEvent {}

packages/core-integration-tests/src/typedef-tests.ts

@@ -0,0 +1,33 @@
+import { CoreAnalyticsTraits, CoreExtraContext } from '@segment/analytics-core'
+
+class TestClass {
+  name = 'hello'
+  toJSON() {
+    return this.name
+  }
+}
+
+export default {
+  'CoreAnalyticsTraits test': () => {
+    let traits: CoreAnalyticsTraits = {}
+
+    // should accept a class
+    traits = new TestClass()
+    // should be nullable
+    traits = { address: null }
+    // should accept aribtrary properties
+    traits = { address: {}, foo: 123 }
+    // should fail with type conflicts
+    // @ts-expect-error
+    traits = { address: 'hello' }
+  },
+
+  'CoreExtraContext test': () => {
+    // should accept a class
+    let ec: CoreExtraContext = {}
+    ec = new TestClass()
+    // should error if type conflict
+    // @ts-expect-error
+    ec = { page: { path: {} } }
+  },
+}