diff --git a/dev-packages/browser-integration-tests/suites/public-api/setTag/with_non_primitives/test.ts b/dev-packages/browser-integration-tests/suites/public-api/setTag/with_non_primitives/test.ts index 648187404c0e..465c2f684de0 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/setTag/with_non_primitives/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/setTag/with_non_primitives/test.ts @@ -3,16 +3,21 @@ import type { Event } from '@sentry/core'; import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; -sentryTest('[bug] accepts non-primitive tags', async ({ getLocalTestUrl, page }) => { - // this is a bug that went unnoticed due to type definitions and a bad assertion - // TODO: We should not accept non-primitive tags. Fix this as a follow-up. +sentryTest('accepts and sends non-primitive tags', async ({ getLocalTestUrl, page }) => { + // Technically, accepting and sending non-primitive tags is a specification violation. + // This slipped through because a previous version of this test should have ensured that + // we don't accept non-primitive tags. However, the test was flawed. + // Turns out, Relay and our product handle invalid tag values gracefully. + // Our type definitions for setTag(s) also only allow primitive values. + // Therefore (to save some bundle size), we'll continue accepting and sending non-primitive + // tag values for now (but not adjust types). + // This test documents this decision, so that we know why we're accepting non-primitive tags. const url = await getLocalTestUrl({ testDir: __dirname }); const eventData = await getFirstSentryEnvelopeRequest(page, url); expect(eventData.message).toBe('non_primitives'); - // TODO: This should be an empty object but instead, it is: expect(eventData.tags).toEqual({ tag_1: {}, tag_2: [], diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 3287d8efbbbd..b23b01664431 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -291,9 +291,7 @@ export class Scope { * Set a single tag that will be sent as tags data with the event. */ public setTag(key: string, value: Primitive): this { - this._tags = { ...this._tags, [key]: value }; - this._notifyScopeListeners(); - return this; + return this.setTags({ [key]: value }); } /** diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts index 280ba4c651ff..221ac14a6fa2 100644 --- a/packages/core/test/lib/scope.test.ts +++ b/packages/core/test/lib/scope.test.ts @@ -140,16 +140,47 @@ describe('Scope', () => { expect(scope['_extra']).toEqual({ a: undefined }); }); - test('setTag', () => { - const scope = new Scope(); - scope.setTag('a', 'b'); - expect(scope['_tags']).toEqual({ a: 'b' }); + describe('setTag', () => { + it('sets a tag', () => { + const scope = new Scope(); + scope.setTag('a', 'b'); + expect(scope['_tags']).toEqual({ a: 'b' }); + }); + + it('sets a tag with undefined', () => { + const scope = new Scope(); + scope.setTag('a', 'b'); + scope.setTag('a', undefined); + expect(scope['_tags']).toEqual({ a: undefined }); + }); + + it('notifies scope listeners once per call', () => { + const scope = new Scope(); + const listener = vi.fn(); + + scope.addScopeListener(listener); + scope.setTag('a', 'b'); + scope.setTag('a', 'c'); + + expect(listener).toHaveBeenCalledTimes(2); + }); }); - test('setTags', () => { - const scope = new Scope(); - scope.setTags({ a: 'b' }); - expect(scope['_tags']).toEqual({ a: 'b' }); + describe('setTags', () => { + it('sets tags', () => { + const scope = new Scope(); + scope.setTags({ a: 'b', c: 1 }); + expect(scope['_tags']).toEqual({ a: 'b', c: 1 }); + }); + + it('notifies scope listeners once per call', () => { + const scope = new Scope(); + const listener = vi.fn(); + scope.addScopeListener(listener); + scope.setTags({ a: 'b', c: 'd' }); + scope.setTags({ a: 'e', f: 'g' }); + expect(listener).toHaveBeenCalledTimes(2); + }); }); test('setUser', () => {