-
Notifications
You must be signed in to change notification settings - Fork 44
Remove the api
package to use concrete common
types #898
Description
Feature Description
In Go, generally, interfaces are small and focused, as well as defined where they're being used. This also helps in testing, where we can define smaller focused interfaces which only mock methods that are used by the test, rather than a large interface where everything needs to be mocked even if majority of the methods aren't being used by the test.
The api
package was useful when we required the concrete types to comply with the interface since we were returning the interfaces to the goja layer directly. E.g.:
// Ensure Browser implements the EventEmitter and Browser interfaces.
var (
_ EventEmitter = &Browser{}
_ api.Browser = &Browser{}
)
Now that we're working with the mapping layer which explicitly maps the implementation to a goja type, this api compliance requirement is no longer required.
When working on recent investigative issues, we've found that this interface api compliance has become a hinderance.
This issue is too explore whether we can remove the api
package and what the consequences of that would be. Something to consider are:
- Is is straight forward to find and replace the interface type with the concrete type?
- How the testing of the mapping layer is affected.
- Will the removal affect future work when working with other web browser?
POC
https://github.com/grafana/xk6-browser/commits/refactor/api-package-poc
### Primary tasks
- [x] grafana/xk6-browser#935
- [ ] grafana/xk6-browser#1055
- [ ] grafana/xk6-browser#1057
### [#1055: Remove the `api` package](https://github.com/grafana/xk6-browser/pull/1055)
- [x] Postfix the `api` types with `API`
- [x] Move the `api` types into the `common` pkg
- [x] Remove the `api` pkg for good
### [#1057: Refactor the `common` API types to concrete](https://github.com/grafana/xk6-browser/pull/1057)
- [x] Remove the `common` pkg API type dependencies
- [x] Use only concrete types throughout the module
- [x] Move the interfaces into the mapping layer and unexport
Already existing or connected issues / PRs (optional)
Related issues: