core: move to src, add TypeScript and rollup #2296

djskinner · 2025-01-28T17:20:11Z

Goal

Convert @bugsnag/core to TypeScript and bundling with rollup in a series of smaller PRs that are easy to review.

The bulk of this PR is to move source code from the root of the package to src whilst allowing existing imports to work (still pointing to the source code) using package.exports.

TypeScript and rollup config and a build step is added but the dist assets are not yet used.

github-actions · 2025-01-28T17:24:38Z

`@bugsnag/browser` bundle size diff

	Minified	Minfied + Gzipped
Before	`50.02 kB`	`15.41 kB`
After	`50.02 kB`	`15.41 kB`
±	No change	No change

code coverage diff

<temporarily disabled>

Generated by 🚫 dangerJS against 63df48b

djskinner · 2025-01-30T16:06:36Z

bin/local-test-util

-    await ex(`npm`, [ `pack`, `--verbose`, `packages/${n}/` ])
+    let packageLocation = `packages/${n}/`
+    if (n === 'plugin-angular') packageLocation += 'dist/'
+    await ex(`npm`, [ `pack`, `--verbose`, packageLocation ])


cherry pick from #2300

djskinner · 2025-01-31T10:06:37Z

jest.config.js

@@ -1,6 +1,7 @@
 const testsForPackage = (packageName) => `<rootDir>/packages/${packageName}/**/*.test.[jt]s?(x)`

 const project = (displayName, packageNames, config = {}) => ({
+  resolver: '<rootDir>/jest/node-exports-resolver',


the jest version being used doesn't support package.exports so we need to use a custom resolver. This is largely based on https://www.npmjs.com/package/jest-node-exports-resolver but required further changes:

Using pkg-up to find the right package json for monorepo packages as the existing logic wasn't working

Setting some default "conditions" as jest-node-exports-resolver out of the box no longer supports our version of jest (see Doesn't work for older versions of Jest k-g-a/jest-node-exports-resolver#17)

An attempt to update jest was previously made (#2231) but there were issues because the helpers we use to run electron tests are no longer maintained and do not work with newwer versions of jest.

As alluded to in the previous PR its possible we could work around this by using multiple versions of jest (if a particular package needs a different version temporarily) rather than the single centralized way it is managed now.

I personally prefer the de-centralised approach to running the tests, but I think it was harder to track changes in test coverage? if we can get around that I'd be happy to take that approach in the future

djskinner · 2025-01-31T10:08:40Z

packages/core/package.json

@@ -1,8 +1,40 @@
 {
  "name": "@bugsnag/core",
-  "main": "index.js",
+  "main": "src/index.js",


In this PR the package source is moved to a src folder but the package exports are still referring to the source (not the generated TypeScript). This allows the file move to be separated from the TypeScript conversion allowing for smaller PRs that are easier to review

djskinner · 2025-01-31T10:11:06Z

packages/core/package.json

  "version": "8.1.1",
  "types": "types/index.d.ts",
+  "exports": {


due to the introduction of a src folder existing import references such as @bugsnag/core/lib/es-utils/map require an export map for them to point to the correct folder (src for now, will migrate to dist is subsequent PRs)

Presumably we'll want to get back to exporting/importing everything from just @bugsnag/core in subsequent PRs

djskinner · 2025-01-31T10:12:02Z

packages/core/package.json

+  "scripts": {
+    "size": "../../bin/size dist/bugsnag.min.js",
+    "clean": "rm -fr dist && mkdir dist",
+    "build": "npm run clean && npm run build:npm",
+    "build:npm": "rollup --config rollup.config.npm.mjs"
+  },


TypeScript compilation is added (with support for .js files) to the build step but the dist assets are not yet being used

djskinner · 2025-01-31T10:13:22Z

packages/core/tsconfig.json

+  "extends": "../../tsconfig.json",
+  "compilerOptions": {
+    "lib": [ "dom", "es2022" ],               /* Specify library files to be included in the compilation. */
+    "allowJs": true,


note: allowJs true

djskinner · 2025-01-31T10:13:53Z

packages/core/tsconfig.json

+  "compilerOptions": {
+    "lib": [ "dom", "es2022" ],               /* Specify library files to be included in the compilation. */
+    "allowJs": true,
+    "outDir": "dist",


I was having some issues with allowJs and for unclear reasons specifying this solved them. See https://stackoverflow.com/questions/42609768/typescript-error-cannot-write-file-because-it-would-overwrite-input-file

djskinner · 2025-01-31T10:14:40Z

tsconfig.json

@@ -1,9 +1,11 @@
 {
  "compilerOptions": {
    "target": "es5",
-    "module": "commonjs",
+    "module": "es2015",
+    "moduleResolution": "bundler",


this allows TypeScript to work with package.exports

https://stackoverflow.com/questions/70296652/how-can-i-use-exports-in-package-json-for-nested-submodules-and-typescript

djskinner · 2025-02-03T16:09:31Z

packages/node/src/bugsnag.ts

@@ -6,8 +6,6 @@ import assign from '@bugsnag/core/lib/es-utils/assign'
 import { schema as baseConfig } from '@bugsnag/core/config'
 import browserConfig from './config'

-import Event from '@bugsnag/core/event'


Using this export from '@bugsnag/core/event' was causing '@bugsnag/core/event' to appear in the generated types, which was causing issues with some older versions of TypeScript because it would need to resort to package.exports, which requires a certain version of TypeScript and moduleResolution: bundler.

I get the feeling a lot of these have come from auto-complete

Yeah, it would be good to get around preventing invalid imports with eslint rules as we do on the dashboard

gingerbenw · 2025-02-10T15:31:36Z

packages/core/rollup.config.npm.mjs

+]
+
+const external = [
+  // '@bugsnag/cuid',


commented out code - is this being added back in for a subsequent PR or can this just be an empty array?

see https://github.com/bugsnag/bugsnag-js/pull/2316/files#r1949315462

gingerbenw · 2025-02-10T15:33:10Z

packages/node/src/bugsnag.ts

@@ -6,8 +6,6 @@ import assign from '@bugsnag/core/lib/es-utils/assign'
 import { schema as baseConfig } from '@bugsnag/core/config'
 import browserConfig from './config'

-import Event from '@bugsnag/core/event'


I get the feeling a lot of these have come from auto-complete

core: move tests from src to test

Dan Skinner added 2 commits January 28, 2025 17:16

core: move to src, add TypeScript and rollup

7a50c55

core: move to src, add TypeScript and rollup

4fd507b

Dan Skinner added 8 commits January 28, 2025 17:29

core: move to src, add TypeScript and rollup

8c184e9

core: move to src, add TypeScript and rollup

0364394

core: move to src, add TypeScript and rollup

9d14f59

add export @bugsnag/core/clone-client

12ddb22

add jest custom resolver to handle packager.exports field

92d4e9e

copy jest dir into container

07cb860

fix test:types

43995bc

remove jest-node-exports-resolver

2235492

Base automatically changed from plat-13218 to integration/typescript January 29, 2025 17:59

Dan Skinner and others added 4 commits January 30, 2025 10:42

Merge branch 'integration/typescript' into plat-12947

076402b

add missing types package to test fixture

5765a17

test: 🩹 fix location when packing plugin-angular

5c128f3

add missing types package to test fixture

1a68b71

djskinner commented Jan 30, 2025

View reviewed changes

Merge branch 'bump-typescript' into plat-12947

5249614

djskinner changed the base branch from integration/typescript to bump-typescript January 31, 2025 10:00

djskinner commented Jan 31, 2025

View reviewed changes

update Dockerfile.ci eslint config location

3bd3332

Base automatically changed from bump-typescript to integration/typescript January 31, 2025 13:48

Merge branch 'dash-13423-2' into plat-12947

cb227a1

djskinner changed the base branch from integration/typescript to dash-13423-2 January 31, 2025 17:36

Dan Skinner added 4 commits January 31, 2025 17:48

fix imports

470f373

fix imports

c46c43a

fix jest resolution

bab00de

Merge branch 'dash-13423-2' into plat-12947

4bea0c0

djskinner mentioned this pull request Feb 3, 2025

node: TypeScript and rollup #2286

Merged

Dan Skinner added 3 commits February 3, 2025 14:52

Merge branch 'dash-13423-2' into plat-12947

13e28c4

add core to local-test-util install steps

7a842ec

avoid ref to @bugsnag/core/event in generated types

63df48b

djskinner commented Feb 3, 2025

View reviewed changes

core: move tests from src to test

a50c977

djskinner marked this pull request as ready for review February 10, 2025 13:38

djskinner requested a review from gingerbenw February 10, 2025 13:39

gingerbenw approved these changes Feb 10, 2025

View reviewed changes

Base automatically changed from dash-13423-2 to integration/typescript February 11, 2025 08:08

Dan Skinner and others added 2 commits February 11, 2025 08:22

Merge branch 'integration/typescript' into plat-12947

24f08b7

Merge pull request #2304 from bugsnag/plat-12947-3

ae50436

core: move tests from src to test

djskinner merged commit bb285ca into integration/typescript Feb 11, 2025
55 checks passed

djskinner deleted the plat-12947 branch February 11, 2025 09:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: move to src, add TypeScript and rollup #2296

core: move to src, add TypeScript and rollup #2296

djskinner commented Jan 28, 2025 •

edited

Loading

github-actions bot commented Jan 28, 2025 •

edited

Loading

djskinner Jan 30, 2025

djskinner Jan 31, 2025

gingerbenw Feb 10, 2025

djskinner Jan 31, 2025

djskinner Jan 31, 2025

djskinner Jan 31, 2025

djskinner Jan 31, 2025

djskinner Jan 31, 2025

djskinner Jan 31, 2025

djskinner Jan 31, 2025

djskinner Feb 3, 2025

gingerbenw Feb 10, 2025

djskinner Feb 11, 2025

gingerbenw Feb 10, 2025

djskinner Feb 11, 2025

gingerbenw Feb 10, 2025

core: move to src, add TypeScript and rollup #2296

core: move to src, add TypeScript and rollup #2296

Conversation

djskinner commented Jan 28, 2025 • edited Loading

Goal

github-actions bot commented Jan 28, 2025 • 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

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 commented Jan 28, 2025 •

edited

Loading

github-actions bot commented Jan 28, 2025 •

edited

Loading

`@bugsnag/browser` bundle size diff