Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drop @bugsnag/core/(client|event|session) exports #2317

Merged
merged 8 commits into from
Feb 14, 2025
Merged

drop @bugsnag/core/(client|event|session) exports #2317

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
944ebf1
remove @bugsnag/core/client export
Feb 10, 2025
50bebe4
remove es3ify-webpack-plugin
Feb 11, 2025
a1d1a39
combine method description
Feb 11, 2025
57b0f2d
Merge branch 'plat-12947-5' into plat-12947-6
Feb 12, 2025
bd375c7
remove @bugsnag/core/(event|session) export
Feb 12, 2025
2264e53
remove @bugsnag/core/(client|config) export
Feb 12, 2025
457bb7b
Revert "remove es3ify-webpack-plugin"
Feb 12, 2025
dc7c41e
Merge branch 'integration/typescript' into plat-12947-6
Feb 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions packages/browser/src/bugsnag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import ClientWithInternals from '@bugsnag/core/client'
import type { BugsnagStatic, Config, Client } from '@bugsnag/core'
import { BugsnagStatic, Config, Client, schema as baseConfig } from '@bugsnag/core'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also exporting schema now


import map from '@bugsnag/core/lib/es-utils/map'
import keys from '@bugsnag/core/lib/es-utils/keys'
import assign from '@bugsnag/core/lib/es-utils/assign'

// extend the base config schema with some browser-specific options
import { schema as baseConfig } from '@bugsnag/core/config'
import browserConfig from './config'

import pluginWindowOnerror from '@bugsnag/plugin-window-onerror'
Expand Down Expand Up @@ -55,10 +53,10 @@ declare global {
}
}

type BrowserClient = Partial<ClientWithInternals> & {
_client: ClientWithInternals | null
type BrowserClient = Partial<Client> & {
_client: Client | null
createClient: (opts?: Config) => Client
start: (opts?: Config) => ClientWithInternals
start: (opts?: Config) => Client
isStarted: () => boolean
}

Expand Down Expand Up @@ -91,7 +89,7 @@ const notifier: BrowserClient = {
]

// configure a client with user supplied options
const bugsnag = new ClientWithInternals(opts, schema, internalPlugins, { name, version, url });
const bugsnag = new Client(opts, schema, internalPlugins, { name, version, url });

// set delivery based on browser capability (IE 8+9 have an XDomainRequest object)
(bugsnag as BrowserClient)._setDelivery?.(window.XDomainRequest ? dXDomainRequest : dXMLHttpRequest)
Expand All @@ -108,17 +106,17 @@ const notifier: BrowserClient = {
notifier._client._logger.warn('Bugsnag.start() was called more than once. Ignoring.')
return notifier._client
}
notifier._client = notifier.createClient(opts) as ClientWithInternals
notifier._client = notifier.createClient(opts)
return notifier._client
},
isStarted: () => {
return notifier._client != null
}
}

type Method = keyof typeof ClientWithInternals.prototype
type Method = keyof typeof Client.prototype

map(['resetEventCount'].concat(keys(ClientWithInternals.prototype)) as Method[], (m) => {
map(['resetEventCount'].concat(keys(Client.prototype)) as Method[], (m) => {
if (/^_/.test(m)) return
notifier[m] = function () {
if (!notifier._client) return console.log(`Bugsnag.${m}() was called before Bugsnag.start()`)
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { schema } from '@bugsnag/core/config'
import { schema } from '@bugsnag/core'
import assign from '@bugsnag/core/lib/es-utils/assign'
import getPrefixedConsole from './get-prefixed-console'

Expand Down
6 changes: 1 addition & 5 deletions packages/browser/src/index-cjs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import Client from '@bugsnag/core/client'
import Event from '@bugsnag/core/event'
import Session from '@bugsnag/core/session'

import { Breadcrumb } from '@bugsnag/core'
import { Breadcrumb, Client, Event, Session } from '@bugsnag/core'

import assign from '@bugsnag/core/lib/es-utils/assign'

Expand Down
6 changes: 1 addition & 5 deletions packages/browser/src/index-umd.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import Client from '@bugsnag/core/client'
import Event from '@bugsnag/core/event'
import Session from '@bugsnag/core/session'

import { Breadcrumb } from '@bugsnag/core'
import { Breadcrumb, Client, Event, Session } from '@bugsnag/core'

import assign from '@bugsnag/core/lib/es-utils/assign'

Expand Down
15 changes: 1 addition & 14 deletions packages/core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,7 @@
"./lib/validators/string-with-length": "./src/lib/validators/string-with-length.js",
"./lib/validators/list-of-functions": "./src/lib/validators/list-of-functions.js",
"./lib/extract-object": "./src/lib/extract-object.js",
"./lib/feature-flag-delegate": "./src/lib/feature-flag-delegate.js",
"./client": {
"import": "./src/client.js",
"default": "./src/client.js",
"types": "./src/client.d.ts"
},
"./config": "./src/config.js",
"./event": {
"import": "./src/event.js",
"default": "./src/event.js",
"types": "./src/event.d.ts"
},
"./session": "./src/session.js"
"./lib/feature-flag-delegate": "./src/lib/feature-flag-delegate.js"
},
"description": "Core classes and utilities for Bugsnag notifiers",
"homepage": "https://www.bugsnag.com/",
Expand All @@ -54,7 +42,6 @@
"files": [
"src",
"types",
"*.js",
"dist"
],
"scripts": {
Expand Down
4 changes: 1 addition & 3 deletions packages/core/rollup.config.npm.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ const plugins = [
babel({ babelHelpers: 'bundled' }),
]

const external = [
'@bugsnag/cuid',
]
const external = [/node_modules/]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as i mentioned in previous PR i think its right that core externalises all its dependencies leaving the individual notifiers to bundle what they need

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be misunderstanding the point of the external dependencies, but why do we need node_modules here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement makes rollup treat all node_modules as external - see https://stackoverflow.com/questions/44844088/how-to-set-as-external-all-node-modules-in-rollup

This means that instead of pulling a load of code into the core bundle they contine to be referenced externally:

const require$$0 = require('error-stack-parser');
const StackGenerator = require('stack-generator');
const require$$2 = require('@bugsnag/safe-json-stringify');
const require$$0$1 = require('iserror');
const cuid = require('@bugsnag/cuid');

// esm version
import require$$0 from 'error-stack-parser';
import StackGenerator from 'stack-generator';
import require$$2 from '@bugsnag/safe-json-stringify';
import require$$0$1 from 'iserror';
import cuid from '@bugsnag/cuid';

The thing is if core copied @bugsnag/cuid into its bundle then a number of other plugins also did this then there would be multiple copies of @bugsnag/cuid code and what's more different instances of it which may cause issues in some situations.

What I'm saying is that for all these shared libraries like core and plugins that are consumed by multiple notifiers they should externalise all their dependencies. Then its left to the notifier builds themselves to pull in a single copy of every external dependency (as you would always want for a browser build). You may or may not want this for a node notifier build, but this gives use the option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, understood! that definitely makes sense then. thanks! 👍🏻


export default [
createRollupConfig({
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export type BreadcrumbType = 'error' | 'log' | 'manual' | 'navigation' | 'process' | 'request' | 'state' | 'user';
1 change: 1 addition & 0 deletions packages/core/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export { default as Breadcrumb } from './breadcrumb'
export { default as Client } from './client'
export { default as Event } from './event'
export { default as Session } from './session'
export { schema } from './config'
8 changes: 4 additions & 4 deletions packages/core/test/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import { BreadcrumbType } from '../types/common'
const noop = () => {}
const id = <T>(a: T) => a

describe('@bugsnag/core/client', () => {
describe('Client', () => {
describe('constructor', () => {
it('can handle bad input', () => {
// @ts-ignore
// @ts-expect-error - testing with unexpected arguments
expect(() => new Client()).toThrow()
// @ts-ignore
// @ts-expect-error - testing with unexpected arguments
expect(() => new Client('foo')).toThrow()
})
})
Expand All @@ -21,7 +21,7 @@ describe('@bugsnag/core/client', () => {
it('handles bad/good input', () => {
expect(() => {
// no opts supplied
// @ts-ignore
// @ts-expect-error - testing with unexpected arguments
const client = new Client({})
expect(client).toBe(client)
}).toThrow()
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import config from '../src/config'

describe('@bugsnag/core/config', () => {
describe('config', () => {
describe('schema', () => {
it('has the required properties { validate(), defaultValue(), message }', () => {
Object.keys(config.schema).forEach(k => {
const key = k as unknown as keyof typeof config.schema
config.schema[key].defaultValue(undefined)
// @ts-expect-error
// @ts-expect-error testing invalid arguments
config.schema[key].validate()
config.schema[key].validate(-1)
config.schema[key].validate('stringy stringerson')
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/event.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ jest.mock('stack-generator', () => ({
backtrace: () => [{}, {}]
}))

describe('@bugsnag/core/event', () => {
describe('Event', () => {
describe('constructor', () => {
it('sets default handledState', () => {
const err = new Error('noooooo')
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/session.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Session from '../src/session'

describe('@bugsnag/core/session', () => {
describe('Session', () => {
describe('toJSON()', () => {
it('returns the correct data structure', () => {
const s = new Session().toJSON()
Expand Down
10 changes: 5 additions & 5 deletions packages/core/types/client.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,26 @@ import {
import Event from './event'
import Session from './session'

interface LoggerConfig {
export interface LoggerConfig {
debug: (msg: any) => void
info: (msg: any) => void
warn: (msg: any) => void
error: (msg: any, err?: unknown) => void
}

interface Notifier {
export interface Notifier {
name: string
version: string
url: string
}

interface EventDeliveryPayload {
export interface EventDeliveryPayload {
apiKey: string
notifier: Notifier
events: Event[]
}

interface SessionDeliveryPayload {
export interface SessionDeliveryPayload {
notifier?: Notifier
device?: Device
app?: App
Expand All @@ -45,7 +45,7 @@ interface SessionDeliveryPayload {
}>
}

interface Delivery {
export interface Delivery {
sendEvent(payload: EventDeliveryPayload, cb: (err?: Error | null) => void): void
sendSession(session: SessionDeliveryPayload, cb: (err?: Error | null) => void): void
}
Expand Down
115 changes: 115 additions & 0 deletions packages/core/types/config.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
export interface Schema {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schema is now being exported from @bugsnag/core - these are the manually crafted types for that (for now) - copied from the internal typings against the js source

apiKey: {
defaultValue: () => null
message: string
validate: (value: unknown) => boolean
}
appVersion: {
defaultValue: () => undefined
message: string
validate: (value: unknown) => boolean
}
appType: {
defaultValue: () => undefined
message: string
validate: (value: unknown) => boolean
}
autoDetectErrors: {
defaultValue: () => true
message: string
validate: (value: unknown) => boolean
}
enabledErrorTypes: {
defaultValue: () => { unhandledExceptions: boolean, unhandledRejections: boolean }
message: string
allowPartialObject: boolean
validate: (value: unknown) => boolean
}
onError: {
defaultValue: () => []
message: string
validate: (value: unknown) => boolean
}
onSession: {
defaultValue: () => []
message: string
validate: (value: unknown) => boolean
}
onBreadcrumb: {
defaultValue: () => []
message: string
validate: (value: unknown) => boolean
}
endpoints: {
defaultValue: (endpoints: { notify: string, sessions: string } | undefined) => { notify: string | null, sessions: string | null }
message: string
validate: (value: unknown) => boolean
}
autoTrackSessions: {
defaultValue: () => boolean
message: string
validate: (value: unknown) => boolean
}
enabledReleaseStages: {
defaultValue: () => null
message: string
validate: (value: unknown) => boolean
}
releaseStage: {
defaultValue: () => 'production'
message: string
validate: (value: unknown) => boolean
}
maxBreadcrumbs: {
defaultValue: () => 25
message: string
validate: (value: unknown) => boolean
}
enabledBreadcrumbTypes: {
defaultValue: () => ['navigation', 'request', 'process', 'log', 'user', 'state', 'error', 'manual']
message: string
validate: (value: unknown) => boolean
}
context: {
defaultValue: () => undefined
message: string
validate: (value: unknown) => boolean
}
user: {
defaultValue: () => {}
message: string
validate: (value: unknown) => boolean
}
metadata: {
defaultValue: () => {}
message: string
validate: (value: unknown) => boolean
}
logger: {
defaultValue: () => undefined
message: string
validate: (value: unknown) => boolean
}
redactedKeys: {
defaultValue: () => ['password']
message: string
validate: (value: unknown) => boolean
}
plugins: {
defaultValue: () => []
message: string
validate: (value: unknown) => boolean
}
featureFlags: {
defaultValue: () => []
message: string
validate: (value: unknown) => boolean
}
reportUnhandledPromiseRejectionsAsHandled: {
defaultValue: () => false
message: string
validate: (value: unknown) => boolean
}
}

export const schema: Schema
36 changes: 36 additions & 0 deletions packages/core/types/event.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import {
Stackframe,
FeatureFlag
} from './common'
import Session from './session'

interface FeatureFlagPayload {
featureFlag: string
variant?: string
}
declare class Event {
public static create(
maybeError: any,
Expand Down Expand Up @@ -56,6 +61,37 @@ declare class Event {

// trace correlation
public setTraceCorrelation(traceId: string, spanId?: string): void

// "private" api
_metadata: { [key: string]: any }
constructor (errorClass: string, errorMessage: string, stacktrace: any[], handledState?: HandledState, originalError?: Error)
_features: FeatureFlag | null[]
_featuresIndex: { [key: string]: number }
_user: User
_handledState: HandledState
_correlation?: { spanId: string, traceId: string }
_session?: Session
toJSON(): {
payloadVersion: '4'
exceptions: Array<Error & { message: Error['errorMessage'] }>
severity: Event['severity']
unhandled: boolean
severityReason: {
type: string
[key: string]: any
}
app: App
device: Device
request: Request
breadcrumbs: Breadcrumb[]
context: string | undefined
correlation: { spanId: string, traceId: string } | undefined
groupingHash: string | undefined
metaData: { [key: string]: any }
user: User
session: Session
featureFlags: FeatureFlagPayload[]
};
}

interface HandledState {
Expand Down
Loading