-
Notifications
You must be signed in to change notification settings - Fork 244
fix(compass-web): bundle type definitions COMPASS-9733 #7227
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds TypeScript type definition bundling for the compass-web package to ensure proper type definitions are available for external consumers. The changes focus on setting up API Extractor to generate clean, consolidated type definitions and implementing proper type safety validation.
- Configures API Extractor to bundle type definitions into a single
compass-web.d.tsfile - Adds type safety testing infrastructure to verify external consumer compatibility
- Exports additional type definitions and marks key functions as public APIs
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compass-web/package.json | Updates build pipeline to include API extractor and type testing |
| packages/compass-web/api-extractor.json | Configures API Extractor for bundling type definitions |
| packages/compass-web/src/index.tsx | Exports additional type definitions for external consumers |
| packages/compass-web/src/entrypoint.tsx | Adds public API annotations and refines type definitions |
| packages/compass-web/src/url-builder.ts | Marks functions as public API |
| packages/compass-web/src/logger.tsx | Marks type definitions as public API |
| packages/compass-web/scripts/clean-dts.mjs | Script to clean up intermediate type definition files |
| packages/compass-web/test/types/ | Test infrastructure to validate type definitions work for external consumers |
| packages/compass-connections/src/stores/store-context.tsx | Fixes void return type issue |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome, thanks for tackling this one! Left two notes: one is tiny nit, another should help to resolve the issue in CI, otherwise this looks good to be merged to me
| }, | ||
| "dtsRollup": { | ||
| "enabled": true, | ||
| "untrimmedFilePath": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think not providing a value at all basically has the same effect
| "untrimmedFilePath": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea an old version of api-extractor required this exist, anyway, gone!
| "types": "./dist/index.d.ts", | ||
| "types": "./dist/compass-web.d.ts", | ||
| "scripts": { | ||
| "prepublishOnly": "npm run compile && compass-scripts check-exports-exist", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check in CI is currently failing because we don't have a bootstrap script in compass-web (it was just not needed before). Now that check depends on some assets being produced, we probably should add it. I don't think we need to waste CI (and local bootstrap for that matter) time building compass-web completely, but definitely looks like we'd need types built
| "prepublishOnly": "npm run compile && compass-scripts check-exports-exist", | |
| "bootstrap": "npm run postcompile", | |
| "prepublishOnly": "npm run compile && compass-scripts check-exports-exist", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes