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

switch to generated type declarations #2327

Open
wants to merge 9 commits into
base: integration/typescript
Choose a base branch
from
Open

switch to generated type declarations #2327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
49d3129
switch to generated type declarations
Feb 14, 2025
1653316
Merge branch 'plat-12947-7' into plat-12947-8
Feb 14, 2025
834e673
skip constructor when adding client methods
Feb 14, 2025
b32a393
Merge branch 'integration/typescript' into plat-12947-8
Feb 17, 2025
fc89d05
reinstate map
Feb 17, 2025
43724bd
prefer ts-expect-error
Feb 17, 2025
cdbf54e
Merge branch 'integration/typescript' into plat-12947-8
Feb 17, 2025
d7014b9
prefer ts-expect-error
Feb 17, 2025
2d00ced
remove unused ts directives
Feb 17, 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
21 changes: 10 additions & 11 deletions packages/browser/src/bugsnag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import { BugsnagStatic, Config, Client, schema as baseConfig } from '@bugsnag/core'

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 browserConfig from './config'

Expand All @@ -18,7 +14,6 @@ import pluginConsoleBreadcrumbs from '@bugsnag/plugin-console-breadcrumbs'
import pluginNetworkBreadcrumbs from '@bugsnag/plugin-network-breadcrumbs'
import pluginNavigationBreadcrumbs from '@bugsnag/plugin-navigation-breadcrumbs'
import pluginInteractionBreadcrumbs from '@bugsnag/plugin-interaction-breadcrumbs'
// @ts-ignore
import pluginInlineScriptContent from '@bugsnag/plugin-inline-script-content'
import pluginSession from '@bugsnag/plugin-browser-session'
import pluginIp from '@bugsnag/plugin-client-ip'
Expand All @@ -32,7 +27,7 @@ const name = 'Bugsnag JavaScript'
const version = '__BUGSNAG_NOTIFIER_VERSION__'
const url = 'https://github.com/bugsnag/bugsnag-js'

const schema = assign({}, baseConfig, browserConfig)
const schema = { ...baseConfig, ...browserConfig }

export interface BrowserConfig extends Config {
maxEvents?: number
Expand Down Expand Up @@ -62,6 +57,7 @@ type BrowserClient = Partial<Client> & {

const notifier: BrowserClient = {
_client: null,
// @ts-expect-error
createClient: (opts) => {
// handle very simple use case where user supplies just the api key as a string
if (typeof opts === 'string') opts = { apiKey: opts }
Expand Down Expand Up @@ -89,9 +85,11 @@ const notifier: BrowserClient = {
]

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

// set delivery based on browser capability (IE 8+9 have an XDomainRequest object)
// @ts-expect-error
(bugsnag as BrowserClient)._setDelivery?.(window.XDomainRequest ? dXDomainRequest : dXMLHttpRequest)

bugsnag._logger.debug('Loaded!')
Expand All @@ -114,21 +112,22 @@ const notifier: BrowserClient = {
}
}

type Method = keyof typeof Client.prototype

const clientMethods = Object.getOwnPropertyNames(Client.prototype).concat(['resetEventCount']) as Method[]
const clientMethods = Object.getOwnPropertyNames(Client.prototype).concat(['resetEventCount'])

map(clientMethods, (m) => {
if (/^_/.test(m)) return
clientMethods.map((m) => {
if (/^_/.test(m) || m === 'constructor') return
// @ts-expect-error
notifier[m] = function () {
if (!notifier._client) return console.log(`Bugsnag.${m}() was called before Bugsnag.start()`)
notifier._client._depth += 1
// @ts-expect-error
const ret = notifier._client[m].apply(notifier._client, arguments)
notifier._client._depth -= 1
return ret
}
})

// @ts-expect-error
const Bugsnag = notifier as BrowserBugsnagStatic

export default Bugsnag
12 changes: 6 additions & 6 deletions packages/browser/test/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ function mockFetch (onSessionSend?: SendCallback, onNotifySend?: SendCallback) {
const session = makeMockXHR(onSessionSend)
const notify = makeMockXHR(onNotifySend)

// @ts-ignore
// @ts-expect-error
window.XMLHttpRequest = jest.fn()
.mockImplementationOnce(() => session)
.mockImplementationOnce(() => notify)
.mockImplementation(() => makeMockXHR(() => {}))
// @ts-ignore
// @ts-expect-error
window.XMLHttpRequest.DONE = DONE

return { session, notify }
Expand Down Expand Up @@ -102,7 +102,7 @@ describe('browser notifier', () => {
type: 'state',
message: 'Bugsnag loaded'
}))
expect(event.originalError.message).toBe('123')
expect((event.originalError as Error).message).toBe('123')
})
})

Expand Down Expand Up @@ -204,7 +204,7 @@ describe('browser notifier', () => {
done(err)
}
expect(event.breadcrumbs.length).toBe(0)
expect(event.originalError.message).toBe('123')
expect((event.originalError as Error).message).toBe('123')
expect(event.getMetadata('debug')).toEqual({ foo: 'bar' })
done()
})
Expand Down Expand Up @@ -235,6 +235,7 @@ describe('browser notifier', () => {
it('resets events on pushState', () => {
const Bugsnag = getBugsnag()
const client = Bugsnag.createClient('API_KEY')
// @ts-expect-error
const resetEventCount = jest.spyOn(client, 'resetEventCount')

window.history.pushState('', '', 'new-url')
Expand All @@ -247,6 +248,7 @@ describe('browser notifier', () => {
it('does not reset events on replaceState', () => {
const Bugsnag = getBugsnag()
const client = Bugsnag.createClient('API_KEY')
// @ts-expect-error
const resetEventCount = jest.spyOn(client, 'resetEventCount')

window.history.replaceState('', '', 'new-url')
Expand Down Expand Up @@ -274,12 +276,10 @@ describe('browser notifier', () => {

describe('payload checksum behavior (Bugsnag-Integrity header)', () => {
beforeEach(() => {
// @ts-ignore
window.isSecureContext = true
})

afterEach(() => {
// @ts-ignore
window.isSecureContext = false
})

Expand Down
4 changes: 2 additions & 2 deletions packages/core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
"name": "@bugsnag/core",
"main": "./dist/index.cjs",
"version": "8.2.0",
"types": "types/index.d.ts",
"types": "./dist/types/index.d.ts",
"exports": {
".": {
"types": "./types/index.d.ts",
"types": "./dist/types/index.d.ts",
"import": "./dist/index.mjs",
"default": "./dist/index.cjs"
},
Expand Down
76 changes: 15 additions & 61 deletions packages/core/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,66 +2,22 @@ import configSchema from './config'
import Event from './event'
import Breadcrumb from './breadcrumb'
import Session from './session'
import map from './lib/es-utils/map'
import includes from './lib/es-utils/includes'
import filter from './lib/es-utils/filter'
import reduce from './lib/es-utils/reduce'
import keys from './lib/es-utils/keys'
import assign from './lib/es-utils/assign'
import runCallbacks from './lib/callback-runner'
import metadataDelegate from './lib/metadata-delegate'
import runSyncCallbacks from './lib/sync-callback-runner'
import BREADCRUMB_TYPES from './lib/breadcrumb-types'
import { add, clear, merge } from './lib/feature-flag-delegate'
import { App, BreadcrumbType, Config, Device, FeatureFlag, OnBreadcrumbCallback, OnErrorCallback, OnSessionCallback, Plugin, User } from './common'
import { BreadcrumbType, Config, Delivery, FeatureFlag, LoggerConfig, Notifier, OnBreadcrumbCallback, OnErrorCallback, OnSessionCallback, Plugin, SessionDelegate, User } from './common'

const noop = () => {}

interface LoggerConfig {
debug: (msg: any) => void
info: (msg: any) => void
warn: (msg: any) => void
error: (msg: any, err?: unknown) => void
}
Comment on lines -20 to -25
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One downside to having babel transpile from TypeScript is that it's not type aware. So if you do:

export { LoggerConfig } from './common'

It fails because LoggerConfig is only a type.

Due to our requirement to support lower TS versions we couldn't do export { type LoggerConfig } from './common' which does fix the issue.

So I moved all the types to common.js which are exported like `export * from './common'`` and there are no complaints.

However, the generated types that TyeScript is now producing still do include the type modifier:

export { default } from './bugsnag';
export type { BrowserBugsnagStatic, BrowserConfig } from './bugsnag';
export * from '@bugsnag/core';

And this can't be solved by us internally not using the type modifier.

To get around this we'll have to use https://www.npmjs.com/package/downlevel-dts as a post-processing step on the types.

An added benefit of this is that internally we'll be able to use type modifier and whatever else we like that is supported by our more recent version of TypeScript

Copy link
Member

Choose a reason for hiding this comment

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

fwiw, we can use the type modifier, but not inline.

export type { LoggerConfig } from './common' works with TS 3.8, but not export { type LoggerConfig } from './common'


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

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

interface SessionDeliveryPayload {
notifier?: Notifier
device?: Device
app?: App
sessions?: Array<{
id: string
startedAt: Date
user?: User
}>
}

interface Delivery {
sendEvent(payload: EventDeliveryPayload, cb: (err?: Error | null) => void): void
sendSession(session: SessionDeliveryPayload, cb: (err?: Error | null) => void): void
}

export interface SessionDelegate<T extends Config = Config> {
startSession: (client: Client<T>, session: Session) => Client
pauseSession: (client: Client<T>) => void
resumeSession: (client: Client<T>) => Client
}

export default class Client<T extends Config = Config> {
private readonly _notifier?: Notifier
public readonly _notifier?: Notifier
// This ought to be Required<T> but the current version of TypeScript doesn't seem to like it
Copy link
Member

Choose a reason for hiding this comment

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

comment can go now

public readonly _config: Required<Config>
public readonly _config: T & Required<Config>
private readonly _schema: any

public _delivery: Delivery
Expand All @@ -87,13 +43,14 @@ export default class Client<T extends Config = Config> {
b: OnBreadcrumbCallback[]
}

private readonly Client: typeof Client
private readonly Event: typeof Event
private readonly Breadcrumb: typeof Breadcrumb
private readonly Session: typeof Session
public readonly Client: typeof Client
public readonly Event: typeof Event
public readonly Breadcrumb: typeof Breadcrumb
public readonly Session: typeof Session

private readonly _depth: number
public _depth: number

public _pausedSession?: Session | null
public _sessionDelegate?: SessionDelegate

constructor (configuration: T, schema = configSchema, internalPlugins: Plugin<T>[] = [], notifier?: Notifier) {
Expand Down Expand Up @@ -139,7 +96,7 @@ export default class Client<T extends Config = Config> {
this.Session = Session

this._config = this._configure(configuration, internalPlugins)
map(internalPlugins.concat(this._config.plugins), pl => {
internalPlugins.concat(this._config.plugins).map(pl => {
if (pl) this._loadPlugin(pl)
})

Expand Down Expand Up @@ -195,7 +152,7 @@ export default class Client<T extends Config = Config> {
this._context = c
}

_configure (opts: T, internalPlugins: Plugin[]) {
_configure (opts: T, internalPlugins: Plugin<T>[]) {
const schema = reduce(internalPlugins, (schema, plugin) => {
if (plugin && plugin.configSchema) return assign({}, schema, plugin.configSchema)
return schema
Expand All @@ -207,7 +164,7 @@ export default class Client<T extends Config = Config> {
}

// accumulate configuration and error messages
const { errors, config } = reduce(keys(schema), (accum, key: keyof typeof opts) => {
const { errors, config } = reduce(Object.keys(schema) as unknown as (keyof T)[], (accum, key: keyof typeof opts) => {
const defaultValue = schema[key].defaultValue(opts[key])

if (opts[key] !== undefined) {
Expand Down Expand Up @@ -249,7 +206,7 @@ export default class Client<T extends Config = Config> {
if (config.onSession) this._cbs.s = this._cbs.s.concat(config.onSession)

// finally warn about any invalid config where we fell back to the default
if (keys(errors).length) {
if (Object.keys(errors).length) {
this._logger.warn(generateConfigErrorMessage(errors, opts))
}

Expand Down Expand Up @@ -369,7 +326,7 @@ export default class Client<T extends Config = Config> {
return types === null || includes(types, type)
}

notify (maybeError: Error, onError?: OnErrorCallback, postReportCallback: (err: Error | null | undefined, event: Event) => void = noop) {
notify (maybeError: Error | string, onError?: OnErrorCallback, postReportCallback: (err: Error | null | undefined, event: Event) => void = noop) {
const event = Event.create(maybeError, true, undefined, 'notify()', this._depth + 1, this._logger)
this._notify(event, onError, postReportCallback)
}
Expand Down Expand Up @@ -411,11 +368,8 @@ export default class Client<T extends Config = Config> {

if (this._isBreadcrumbTypeEnabled('error')) {
// only leave a crumb for the error if actually got sent
// @ts-ignore
Client.prototype.leaveBreadcrumb.call(this, event.errors[0].errorClass, {
// @ts-ignore
errorClass: event.errors[0].errorClass,
// @ts-ignore
errorMessage: event.errors[0].errorMessage,
severity: event.severity
}, 'error')
Expand Down Expand Up @@ -447,7 +401,7 @@ export default class Client<T extends Config = Config> {

const generateConfigErrorMessage = (errors: Record<string, Error>, rawInput: Config) => {
const er = new Error(
`Invalid configuration\n${map(keys(errors), key => ` - ${key} ${errors[key]}, got ${stringify(rawInput[key])}`).join('\n\n')}`)
`Invalid configuration\n${(Object.keys(errors) as unknown as (keyof Config)[]).map(key => ` - ${key} ${errors[key]}, got ${stringify(rawInput[key])}`).join('\n\n')}`)
return er
}

Expand Down
56 changes: 49 additions & 7 deletions packages/core/src/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ export interface Logger {
error: (...args: any[]) => void
}

export interface SessionDelegate {
startSession: (client: Client) => Client
}

export interface EventPayload {
apiKey: string
notifier: {
Expand Down Expand Up @@ -130,9 +126,9 @@ export interface Request {
}

export interface User {
id?: string | null
email?: string | null
name?: string | null
id?: string | null | undefined
email?: string | null | undefined
name?: string | null | undefined
Comment on lines +129 to +131
Copy link
Member

Choose a reason for hiding this comment

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

doesn't the ? cover undefined already?

}

type ThreadType = 'cocoa' | 'android' | 'browserJs'
Expand All @@ -157,4 +153,50 @@ export interface Stackframe {
export interface FeatureFlag {
name: string
variant?: string | null
}

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

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

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

export interface SessionDeliveryPayload {
notifier?: Notifier
device?: Device
app?: App
sessions?: Array<{
id: string
startedAt: Date
user?: User
}>
}
export interface Delivery {
sendEvent(payload: EventDeliveryPayload, cb: (err?: Error | null) => void): void
sendSession(session: SessionDeliveryPayload, cb: (err?: Error | null) => void): void
}

export interface SessionDelegate<T extends Config = Config> {
startSession: (client: Client<T>, session: Session) => Client
pauseSession: (client: Client<T>) => void
resumeSession: (client: Client<T>) => Client
}

export interface BugsnagStatic extends Client {
start(apiKeyOrOpts: string | Config): Client
createClient(apiKeyOrOpts: string | Config): Client
isStarted(): boolean
}
Loading