Skip to content

Conversation

lightning-sagar
Copy link

Summary

This PR fixes incorrect type imports in the @stream-io/video-react-sdk.
All type-only imports now use import type instead of a regular import, following TypeScript best practices.

Changes

  • Updated type imports in SDK source files

Why

  • Ensures types are correctly stripped from compiled JavaScript
  • Improves compatibility with bundlers (Vite, Next.js)
  • Aligns with TypeScript best practices

Closes #1928

@oliverlaz
Copy link
Member

hi @lightning-sagar, thank you for your effort and contribution. Before moving forward I'd like to ask you sign this CLA:

Afterwards, to make this future-prone and performant, can you please update our TS config files and enable the verbatimModuleSyntax?

I expect yarn build:all to start failing for other packages, but once everything is green, we can move forward and accept this PR.

@lightning-sagar
Copy link
Author

Hi @oliverlaz, I’ve addressed the changes you requested... however, after running yarn build:all, there are a lot of unused variable warnings across the SDK... should I leave them as is for now? as for now it didn't block the build.

@lightning-sagar
Copy link
Author

Hi @oliverlaz , I’ve pushed the latest changes,everything is now updated as requested...
I ran yarn build:all and it passes successfully without failures.

⚠️ There are still warnings about unused variables/components across the packags, but they don’t block or break the build...

@oliverlaz
Copy link
Member

hey @lightning-sagar, I don't expect to see "unused variables" issues. Can you please share a sample output here?

@lightning-sagar
Copy link
Author

hi @oliverlaz , sorry for the earlier confusion! just to clarify, the warnings I mentioned are related to unused external imports (e.g., Observable, Subject from rxjs, and UnaryCall from @protobuf-ts/runtime-rpc) these are showing up during the build but dont block or break it...

✅ I’ve completed all requested changes:

  • Updated all type-only imports to use import type
  • Enabled verbatimModuleSyntax in the TS config
  • Ran yarn build:all, all packages build successfully with no errors...

is there anything else you'd like me to address, or are we good to proceed with the review/approval???

@lightning-sagar
Copy link
Author

hi @oliverlaz ,
I’ve made all the requested changes (type-only imports, verbatimModuleSyntax, and confirmed yarn build:all passes)
Is the PR ready for approval, or would you like me to address anything else?

Thanks!

@oliverlaz
Copy link
Member

Hi @lightning-sagar, thanks for your contributions.
I'll review this PR in more detail later this week, and if everything is good, I'll go ahead and merge it.

@oliverlaz oliverlaz self-assigned this Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect usage of import instead of import type for type-only imports in documentation and source code
2 participants