Skip to content
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

Expose EmittedEvents subtypes #3506

Closed
farooqkz opened this issue Jun 25, 2023 · 9 comments · Fixed by #3597
Closed

Expose EmittedEvents subtypes #3506

farooqkz opened this issue Jun 25, 2023 · 9 comments · Fixed by #3597
Assignees

Comments

@farooqkz
Copy link

Currently it is a compound of these:

ClientEvent | RoomEvents | RoomStateEvents | CryptoEvents | MatrixEventEvents | RoomMemberEvents | UserEvents | CallEvent | Incoming | Incoming | Outgoing | Ended | Participants | SessionLoggedOut | NoConsent | BeaconEvent

But few of them are exported and many like CallEvent, Incoming and RoomEvent are not exported and therefore cannot be used. And we have to hardcode their values in our apps rather than use these enums which is the right way.

@richvdh
Copy link
Member

richvdh commented Jun 26, 2023

It's a general theme of matrix-js-sdk that it doesn't correctly export types that are meant to be public.

You can generally work around it by directly including the file where they are declared. For example, in the case of CallEvent, you can try:

import type { CallEvent } from "matrix-js-sdk/src/webrtc/call";

@farooqkz
Copy link
Author

It's a general theme of matrix-js-sdk that it doesn't correctly export types that are meant to be public.

You can generally work around it by directly including the file where they are declared. For example, in the case of CallEvent, you can try:

import type { CallEvent } from "matrix-js-sdk/src/webrtc/call";

Thanks for your reply with workaround. While what you suggest works, I believe the SDK still must expose CallEvent and others like it exposes other things.

@richvdh
Copy link
Member

richvdh commented Jun 26, 2023

Totally agree. Importing things from individual files is just a workaround.

@davidisaaclee
Copy link
Contributor

In my setup (Webpack 5 via create-react-app), importing an enum from a specific file (e.g. import { ThreadEvent } from 'matrix-js-sdk/src/models/Thread') causes lots of build errors. I'm bad at JS build tools, so I'm not entirely sure what the problem is, but it sounds to me like the import is triggering Webpack to try to parse the referenced file, which fails.

I'm working around these issues by (1) using literal strings instead of enum members ('Thread.new' as any), and (2) maintaining a fork.

tbc, would PRs exposing some of these enums / types be welcome?

@richvdh
Copy link
Member

richvdh commented Jun 28, 2023

Yes, I'd be very happy to review PRs exposing these enums/types.

@farooqkz
Copy link
Author

In my setup (Webpack 5 via create-react-app), importing an enum from a specific file (e.g. import { ThreadEvent } from 'matrix-js-sdk/src/models/Thread') causes lots of build errors. I'm bad at JS build tools, so I'm not entirely sure what the problem is, but it sounds to me like the import is triggering Webpack to try to parse the referenced file, which fails.

I'm working around these issues by (1) using literal strings instead of enum members ('Thread.new' as any), and (2) maintaining a fork.

tbc, would PRs exposing some of these enums / types be welcome?

I have similar problems in my setup too when importing from individual SDK files. I'm using typescript to using strings directly is another problem...

@t3chguy
Copy link
Member

t3chguy commented Jun 28, 2023

@davidisaaclee you likely want to import it from /lib/ rather than /src/

@davidisaaclee
Copy link
Contributor

@t3chguy thank you – it works! :)

import { ThreadEvent } from "matrix-js-sdk/lib/models/thread";

@richvdh
Copy link
Member

richvdh commented Jul 11, 2023

But few of them are exported and many like CallEvent, Incoming and RoomEvent are not exported and therefore cannot be used. And we have to hardcode their values in our apps rather than use these enums which is the right way.

Just as a matter of detail: RoomEvent is in fact exported correctly. But CallEvent and Incoming are indeed not.

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 a pull request may close this issue.

4 participants