-
Notifications
You must be signed in to change notification settings - Fork 4
Fix type bundling #45
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
| exclude: ['**/*.test.ts'], | ||
| outDir: 'dist', | ||
| insertTypesEntry: true, | ||
| copyDtsFiles: true, |
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.
This is the essential fix. The .d.ts files are not copied by default, that's why the types are missing.
| beforeWriteFile(filePath, content) { | ||
| return filePath.match(/\/(vite|jest)\./) ? false : { filePath, content }; | ||
| }, |
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.
This parts just removes the types created for the config files to keep things clean. They're not needed for anything
|
Thanks for working on this @danielfdsilva! I've tested this approach and unfortunately the The generated type declarations still contain interface inheritance: // dist/src/hooks/useItem.d.ts
interface StacItemHook extends StacHook {
item?: Item;
refetch: StacRefetchFn<Item>;
}While the How to Reproduce# Checkout this branch
git checkout fix/ts.d
# Build the package
yarn build
# Check the generated type declarations
cat dist/src/hooks/useItem.d.ts
# You'll see: interface StacItemHook extends StacHook
# The base interface exists but isn't flattened
cat dist/src/types/index.d.ts
# You'll see the StacHook interface definitionThe issue is that interface inheritance isn't being flattened in the declaration files, which causes problems for consuming projects. Alternative SolutionPR #44 takes a different approach by removing the interface inheritance pattern entirely and using explicit type definitions. This ensures all properties are visible inline in the generated What Should Be Kept from This PRThis PR has two valuable changes that should be preserved:
I'd suggest we merge PR #44 to fix the inheritance issue, then cherry-pick these two commits in a follow-up PR. |
|
@AliceR This does keep the inheritance interface. That is intentional. It is a standard feature of typescript and I'd say it is working well. I had the error you're mentioning as well and this fixes it.
|
assetsshould be a dict and not an array: https://github.com/radiantearth/stac-spec/blob/master/item-spec/item-spec.md#assets