matrix-org / matrix-org/matrix-js-sdk
Don't mixin MatrixBaseApis into MatrixClient
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 2.2k
- Forks
- 704
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 40
Description
I assume this was done to preserve backwards compat whilst transitioning the HTTP APIs, but there is no visible TODO/XXX/FIXME to say that using the mixin is bad.
It's bad because:
- It's now in the JSDocs, so developers need to look in 2 places to find a function on `MatrixClient` (the base API class and the client class).
- There seems to be no thought to what happens if both classes specify the same function name. No warnings. No thrown errors. Nothing. This would be a pain to debug.
- It encourages tight coupling between the 2 classes. Nothing stops `MatrixBaseApis` from calling functions defined on `MatrixClient` and then breaking horribly when people try to use `MatrixBaseApis` stand-alone.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the MatrixClient and MatrixBaseApis definitions and tracing how the mixin is applied and exposed in the JSDocs. Review the existing HTTP API transition for compatibility expectations; done means the mixin relationship is removed or otherwise resolved without ambiguous overlapping methods or broken standalone use.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100