Refactor @bugsnag/browser #2252

gingerbenw · 2024-11-12T16:18:47Z

Goal

This PR converts the last of the internal browser plugins and the browser client to TypeScript, using Rollup to bundle both common js and es modules

Changeset

Convert browser package to TypeScript
Convert plugin-simple-throttle to TypeScript
Convert plugin-interaction-breadcrumbs to TypeScript
Convert plugin-navigation-breadcrumbs to TypeScript
Create browser bundle for both npm and cdn using rollup

Testing

Covered by existing test suite

github-actions · 2024-12-10T16:43:23Z

`@bugsnag/browser` bundle size diff

	Minified	Minfied + Gzipped
Before	`46.15 kB`	`13.86 kB`
After	`48.86 kB`	`15.14 kB`
±	⚠️ `+2,710 bytes`	⚠️ `+1,275 bytes`

code coverage diff

<temporarily disabled>

Generated by 🚫 dangerJS against 3196415

djskinner · 2024-12-11T15:51:28Z

jest.config.js

@@ -49,7 +49,9 @@ module.exports = {
      'plugin-simple-throttle',
      'plugin-console-breadcrumbs',
      'plugin-browser-session'
-    ]),
+    ], {
+      setupFiles: ['<rootDir>/jest/setup/mockEventTarget.js']


See jsdom/jsdom#2156

This is needed to fix the TypeError: 'addEventListener' called on an object that is not a valid instance of EventTarget. seen in the integration tests following the rollup changes

djskinner · 2024-12-16T13:11:21Z

packages/browser/package.json

    "build:dist": "cross-env NODE_ENV=production bash -c '../../bin/bundle src/notifier.js --standalone=Bugsnag | ../../bin/extract-source-map dist/bugsnag.js'",
    "build:dist:min": "cross-env NODE_ENV=production bash -c '../../bin/bundle src/notifier.js --standalone=Bugsnag | ../../bin/minify dist/bugsnag.min.js'",


can these be removed?

yep - and possibly the minification scripts, too?

Not yet as they are being used for e.g. plugin-react and others

packages/browser/test/index.test.ts

This reverts commit 0052406.

This reverts commit a10a0f8.

djskinner · 2024-12-27T10:31:03Z

packages/browser/src/index-umd.ts

+export default assign(Bugsnag, { Client, Event, Session, Breadcrumb })
+export type { BrowserBugsnagStatic, BrowserConfig } from './bugsnag'
+
+export type { Client, Event, Session, Breadcrumb, Plugin } from '@bugsnag/core'


Ideally we would do export type * from '@bugsnag/core' but that's not supported with the current version of TypeScript we're using

TODO: Check what other types need exporting here

Actually I don't think any types need exporting from cjs/umd entry points

djskinner · 2024-12-27T10:32:45Z

packages/browser/src/bugsnag.ts

+import pluginNavigationBreadcrumbs from '@bugsnag/plugin-navigation-breadcrumbs'
+import pluginInteractionBreadcrumbs from '@bugsnag/plugin-interaction-breadcrumbs'
+// @ts-ignore
+import pluginInlineScriptContent from '@bugsnag/plugin-inline-script-content'


I reverted the TS conversion of this plugin from this PR because it was broken

djskinner · 2024-12-27T10:34:52Z

packages/browser/rollup.config.npm.mjs

+
+const sharedOutput = {
+  ...commonSharedOutput,
+  strict: false, // 'use strict' in WebKit enables Tail Call Optimization, which breaks stack trace handling


There is one tail call in out stack and our depth handling does not handle it. With TCO enabled the stack frame for the tail call is removed, which then causes problems with stack processing leading to unexpected results (only in WebKit browsers since that's the only one supporting TCO):

Dropping 'use strict' prevents TCO being enabled and resolves the issue

This reverts commit 0438ff1.

djskinner · 2024-12-27T16:41:47Z

packages/browser/package.json

+    "@rollup/plugin-node-resolve": "^16.0.0",
+    "@rollup/plugin-replace": "^6.0.2",
+    "@rollup/plugin-terser": "^0.4.4",
+    "tslib": "^2.8.1"


tslib is required for target: es3

gingerbenw · 2025-01-06T11:40:43Z

packages/browser/src/index-cjs.ts

+import Client from '@bugsnag/core/client'
+// @ts-ignore
+import Event from '@bugsnag/core/event'
+// @ts-ignore
+import Session from '@bugsnag/core/session'
+// @ts-ignore
+import Breadcrumb from '@bugsnag/core/breadcrumb'


Assuming these ignores will be removed once we've finished conversion of the core package

gingerbenw · 2025-01-06T11:44:00Z

packages/plugin-interaction-breadcrumbs/src/interaction-breadcrumbs.ts

@@ -1,10 +1,13 @@
+import { Plugin } from '@bugsnag/core'
+import type ClientWithInternals from 'packages/core/client'


so this import is working fine?

This one happens to work fine because that import path doesn't appear in the final generated types:

import { Plugin } from '@bugsnag/core'; declare const _default: (win?: Window & typeof globalThis) => Plugin; export default _default; //# sourceMappingURL=interaction-breadcrumbs.d.ts.map

gingerbenw · 2025-01-06T11:44:20Z

packages/plugin-interaction-breadcrumbs/src/interaction-breadcrumbs.ts

@@ -23,7 +26,8 @@ module.exports = (win = window) => ({

 const trim = /^\s*([^\s][\s\S]{0,139}[^\s])?\s*/

-function getNodeText (el) {
+// TODO: Fix Type


Want me to take some of these TODOs ?

gingerbenw force-pushed the PLAT-12163/browser branch from a04fe2c to 1b9a68e Compare November 15, 2024 16:33

gingerbenw force-pushed the PLAT-12163/browser branch from a881ec2 to 5519d06 Compare December 4, 2024 17:50

gingerbenw force-pushed the integration/typescript branch from 9a4591e to 7bbbff9 Compare December 5, 2024 16:04

gingerbenw force-pushed the PLAT-12163/browser branch from 5519d06 to 08a3dc9 Compare December 5, 2024 16:49

gingerbenw changed the title ~~Plat 12163/browser~~ Refactor @bugsnag/browser Dec 5, 2024

gingerbenw added 7 commits December 9, 2024 11:55

refactor: ♻️ convert browser package to typescript

3daf716

refactor: ♻️ refactor plugin-navigation-breadcrumbs

a089f1f

refactor: ♻️ refactor plugin-simple-throttle

c55e8e8

refactor: ♻️ refactor plugin-interaction-breadcrumbs

65d11f0

update types

6da01bb

refactor: ♻️ refactor plugin-inline-script-content

7e8257a

fix rollup config

8c78faf

gingerbenw force-pushed the PLAT-12163/browser branch from 08a3dc9 to 8c78faf Compare December 10, 2024 16:39

gingerbenw and others added 3 commits December 11, 2024 09:35

update imports

b813a19

fix import

1c0e1e9

fix dist and tests

a70287f

djskinner reviewed Dec 11, 2024

View reviewed changes

Dan Skinner added 2 commits December 11, 2024 16:00

fix dist and tests

066d17e

fix dist and tests

d2badbd

djskinner reviewed Dec 16, 2024

View reviewed changes

Dan Skinner added 9 commits December 18, 2024 17:03

browser bundling

e21209b

browser bundling

337ca20

browser bundling

7a9ce69

browser bundling

2b89432

browser bundling

2d3049b

browser bundling

2a9479d

browser bundling

1c3b2ac

browser bundling

15ffd87

browser bundling

c8d3e00

Dan Skinner added 10 commits December 20, 2024 15:33

browser bundling

ccd55cf

browser bundling

818ceac

browser bundling

c379891

browser bundling

2aedd15

browser bundling

18c153a

browser bundling

111682e

trigger build

1cb15ff

trigger build

a792936

trigger build

be9d436

browser bundling

21f1b38

djskinner reviewed Dec 23, 2024

View reviewed changes

packages/browser/test/index.test.ts Outdated Show resolved Hide resolved

Dan Skinner added 10 commits December 24, 2024 08:39

revert mockEventTarget

9a0c817

browser bundling

71331ce

browser bundling

0052406

Revert "browser bundling"

a10a0f8

This reverts commit 0052406.

Revert "Revert "browser bundling""

4fb4738

This reverts commit a10a0f8.

browser bundling

64823d5

browser bundling

746d8a4

disable terser

b1bd4f0

browser bundling

f94feff

browser bundling

0438ff1

djskinner reviewed Dec 27, 2024

View reviewed changes

Dan Skinner added 2 commits December 27, 2024 10:38

Revert "browser bundling"

79deeb7

This reverts commit 0438ff1.

browser bundling

63d1290

djskinner reviewed Dec 27, 2024

View reviewed changes

browser bundling

3196415

gingerbenw marked this pull request as ready for review January 6, 2025 10:54

gingerbenw commented Jan 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor @bugsnag/browser #2252

Refactor @bugsnag/browser #2252

gingerbenw commented Nov 12, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading

djskinner Dec 11, 2024

djskinner Dec 16, 2024

gingerbenw Dec 16, 2024

djskinner Dec 27, 2024

djskinner Dec 27, 2024

djskinner Dec 27, 2024

djskinner Dec 27, 2024

djskinner Dec 27, 2024

djskinner Dec 27, 2024

djskinner Dec 27, 2024

gingerbenw Jan 6, 2025

gingerbenw Jan 6, 2025

djskinner Jan 6, 2025 •

edited

Loading

gingerbenw Jan 6, 2025

		"build:dist": "cross-env NODE_ENV=production bash -c '../../bin/bundle src/notifier.js --standalone=Bugsnag \| ../../bin/extract-source-map dist/bugsnag.js'",
		"build:dist:min": "cross-env NODE_ENV=production bash -c '../../bin/bundle src/notifier.js --standalone=Bugsnag \| ../../bin/minify dist/bugsnag.min.js'",

		@@ -1,10 +1,13 @@
		import { Plugin } from '@bugsnag/core'
		import type ClientWithInternals from 'packages/core/client'

Refactor @bugsnag/browser #2252

Are you sure you want to change the base?

Refactor @bugsnag/browser #2252

Conversation

gingerbenw commented Nov 12, 2024 • edited Loading

Goal

Changeset

Testing

github-actions bot commented Dec 10, 2024 • edited Loading

@bugsnag/browser bundle size diff

code coverage diff

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djskinner Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gingerbenw commented Nov 12, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading

`@bugsnag/browser` bundle size diff

djskinner Jan 6, 2025 •

edited

Loading