Conversation
There was a problem hiding this comment.
Pull request overview
Updates the @bugsnag/web-worker published type entrypoint to use a type-only export for WorkerConfig, avoiding TS1205 when consumers compile with --isolatedModules (e.g. Vite / single-file transpilers).
Changes:
- Switch
WorkerConfigexport inpackages/web-worker/types/notifier.tstoexport type { WorkerConfig }. - Add a new Jest test file intended to validate
WorkerConfigtyping expectations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/web-worker/types/notifier.ts | Uses a type-only export for WorkerConfig to support --isolatedModules. |
| packages/web-worker/types/notifier.test.ts | Adds a test intended to enforce WorkerConfig type behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe('WorkerConfig type', () => { | ||
| it('should allow apiKey and custom fields', () => { | ||
| // This should type-check without error | ||
| const config: WorkerConfig = { | ||
| apiKey: '123', | ||
| collectUserIp: true, | ||
| generateAnonymousId: false | ||
| }; | ||
| expect(config.apiKey).toBe('123'); | ||
| expect(config.collectUserIp).toBe(true); | ||
| expect(config.generateAnonymousId).toBe(false); | ||
| }); | ||
|
|
||
| it('should error if apiKey is missing', () => { | ||
| // @ts-expect-error: apiKey is required | ||
| const config: WorkerConfig = { | ||
| collectUserIp: true, | ||
| generateAnonymousId: false | ||
| }; | ||
| expect(config).toBeDefined(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This test file lives under packages/web-worker/types/, and packages/web-worker/package.json publishes the entire types directory ("files": ["dist", "types"]). That means this *.test.ts will be shipped to consumers. Consider moving this test into packages/web-worker/test/ (or excluding **/*.test.* from the published files list) so the published package only contains actual type entrypoints.
| describe('WorkerConfig type', () => { | |
| it('should allow apiKey and custom fields', () => { | |
| // This should type-check without error | |
| const config: WorkerConfig = { | |
| apiKey: '123', | |
| collectUserIp: true, | |
| generateAnonymousId: false | |
| }; | |
| expect(config.apiKey).toBe('123'); | |
| expect(config.collectUserIp).toBe(true); | |
| expect(config.generateAnonymousId).toBe(false); | |
| }); | |
| it('should error if apiKey is missing', () => { | |
| // @ts-expect-error: apiKey is required | |
| const config: WorkerConfig = { | |
| collectUserIp: true, | |
| generateAnonymousId: false | |
| }; | |
| expect(config).toBeDefined(); | |
| }); | |
| }); | |
| // This file intentionally performs compile-time validation for the public type. | |
| // It avoids runtime test constructs so it can safely live alongside published | |
| // type entrypoints without depending on a test framework. | |
| const config: WorkerConfig = { | |
| apiKey: '123', | |
| collectUserIp: true, | |
| generateAnonymousId: false | |
| }; | |
| const apiKey: string = config.apiKey; | |
| const collectUserIp: boolean = config.collectUserIp; | |
| const generateAnonymousId: boolean = config.generateAnonymousId; | |
| void apiKey; | |
| void collectUserIp; | |
| void generateAnonymousId; | |
| // @ts-expect-error: apiKey is required | |
| const invalidConfig: WorkerConfig = { | |
| collectUserIp: true, | |
| generateAnonymousId: false | |
| }; | |
| void invalidConfig; | |
| export {}; |
|
|
||
| it('should error if apiKey is missing', () => { | ||
| // @ts-expect-error: apiKey is required | ||
| const config: WorkerConfig = { | ||
| collectUserIp: true, | ||
| generateAnonymousId: false | ||
| }; | ||
| expect(config).toBeDefined(); |
There was a problem hiding this comment.
This test intends to assert type behavior (including @ts-expect-error), but Jest + Babel compilation won’t fail on TypeScript type errors. Also, packages/web-worker is not included in the root tsconfig.json that’s compiled by npm run test:types, so this file likely won’t be type-checked in CI. To make this meaningful, add a type-check step that includes packages/web-worker/types (e.g. a package-level test:types that runs tsc --noEmit over these files, or include this directory in an existing type-test project).
| import type { WorkerConfig } from './notifier'; | ||
|
|
||
| describe('WorkerConfig type', () => { | ||
| it('should allow apiKey and custom fields', () => { | ||
| // This should type-check without error | ||
| const config: WorkerConfig = { | ||
| apiKey: '123', | ||
| collectUserIp: true, | ||
| generateAnonymousId: false | ||
| }; | ||
| expect(config.apiKey).toBe('123'); | ||
| expect(config.collectUserIp).toBe(true); | ||
| expect(config.generateAnonymousId).toBe(false); | ||
| }); |
There was a problem hiding this comment.
This file’s formatting (semicolons, etc.) doesn’t match the repo’s ESLint config (standard-with-typescript), which enforces a no-semicolons style for *.ts/*.test.ts. Running npm run test:lint will likely fail on this file; please align it with the existing test style in packages/web-worker/test/*.ts.
| describe('WorkerConfig type', () => { | ||
| it('should allow apiKey and custom fields', () => { | ||
| // This should type-check without error | ||
| const config: WorkerConfig = { | ||
| apiKey: '123', | ||
| collectUserIp: true, | ||
| generateAnonymousId: false | ||
| }; |
There was a problem hiding this comment.
The test name says it "should allow apiKey and custom fields", but the object literal only uses declared WorkerConfig fields (apiKey, collectUserIp, generateAnonymousId). Either add an actual extra property to demonstrate custom fields (if intended/supported), or rename the test to reflect what it’s asserting.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('should allow apiKey and custom fields', () => { | ||
| // This should type-check without error | ||
| const config: WorkerConfig = { | ||
| apiKey: '123', | ||
| collectUserIp: true, | ||
| generateAnonymousId: false | ||
| }; |
There was a problem hiding this comment.
The test name/comment says “custom fields”, but the object literal only uses known properties (apiKey, collectUserIp, generateAnonymousId). Also, WorkerConfig (via Config) doesn’t appear to allow arbitrary extra keys, so this wording is misleading. Please rename the test to reflect what’s actually being exercised, or add a true “extra property allowed” case if that’s intended behavior.
| import type { WorkerConfig } from './notifier'; | ||
|
|
||
| describe('WorkerConfig type', () => { | ||
| it('should allow apiKey and custom fields', () => { | ||
| // This should type-check without error |
There was a problem hiding this comment.
This test file uses semicolons, which will violate the repo’s Standard/standard-with-typescript lint rules (existing *.test.ts files in this repo omit semicolons). Please align formatting with the project style (remove semicolons, trailing punctuation) to avoid CI lint failures.
Goal
Fix the TypeScript type export in @bugsnag/web-worker to support projects using the --isolatedModules flag (e.g., Vite, modern React setups). This prevents TS1205 errors and ensures type-only exports are compatible with single-file transpilers.
Design
The fix uses the export type { WorkerConfig } syntax, which is required by TypeScript when re-exporting types with --isolatedModules. This is the recommended and future-proof way to export types, ensuring compatibility with all TypeScript build modes.
Changeset
Changed export { WorkerConfig } to export type { WorkerConfig } in notifier.ts.
Added/updated a unit test (notifier.test.ts) to verify correct type export and required properties in WorkerConfig.
Note: test-repro.ts was used for local verification only and is not included in this PR.
Testing
Verified locally with a temporary test file that the TS1205 error is resolved and type checking works as expected.
Added a unit test to ensure WorkerConfig requires apiKey and allows custom fields.
Confirmed that omitting apiKey triggers a type error as expected.