This was not as easy as the other two. The main issue involves the `BaseDriver` class and is threefold:
1. `BaseDriver` and a valid external subclass thereof (a `Driver`) are not the same, because the former is essentially _abstract_. There is no notion of an abstract class in JS nor in TS for JS.
2. Splitting `BaseDriver` into several modules and "mixing in" some modules is just not well-understood by TS.
3. `AppiumDriver` inherits _most_ of `BaseDriver`, but overrides some functions with differing signatures. This is unsupported by TS.
To work around these problems, I had to:
1. Move the `Driver` interface into `@appium/types`, since other packages may depend on it. We do not want circular dependencies, which is likely to occur otherwise. Further, there is no way to tell TS for JS that a class _implements_ an interface or type.
2. Refactor the mixins (in `basedriver/commands/`) into "mixin classes", as understood by typescript. These are dynamic classes returned by functions which accept a class parameter, and _extend_ that class. The result is marginally better, but the implementation is ... well, see for yourself. It is not possible to use declaration merging to just "trick" TS into thinking that `BaseDriver` has all of these extra methods; classes cannot be merged, and even if you could, you can't use declaration merging to "add" methods afaict.
3. `BaseDriver` is now an empty class which inherits from all mixins applied to `DriverCore`. Due to syntactic restrictions, it must be a `class` statement in order to use the `@implements` tag, which causes TS to check that the result fully implements `Driver`. `AppiumDriver` _also_ inherits from `DriverCore`, since it does not override `BaseDriver` "properly". `BaseDriverCore` itself is pretty small, containing only those functions which `AppiumDriver` defines as differing.
Otherwise:
- `BaseDriver` does not inherit from `Protocol`, which is just an empty class. `Protocol` was removed.
- All of the error classes are exported from `protocol/errors`, because TS cannot see them otherwise.
- Exported missing `GET_STATUS_COMMAND` from `protocol/index`
- Re-exported `normalizeBasePath` from `server` instead of `protocol`, since that's where it's defined
- Removed `index.d.ts`
- Mixins use `@implements` as well. Note that `@implements` does not support `import('..')` syntax.
chore(base-driver): attempted to make types for mixins less awful
fixed a couple issues in the types in `driver.js`
core(base-driver): add capabilities types
- also move the bluebird config into the index
- fix references to `DriverOpts`
- add `assignServer()` to `BaseDriverCore`, used by `AppiumDriver`
chore(base-driver): fix more types
add missing reference to `@appium/types` in `tsconfig.json`
fix(base-driver): ensure getLogEvents is async
fix(base-driver): add some more type checks and fix logCustomEvent to be async
chore(base-driver): reformat
Most of these changes are the addition of types. There are a few places in which the typechecker found weirdness, and I have addressed those.
- In `logging.js`, `parseInt()` was being called on props of `process.env`; this is not allowed, because `parseInt()` only accepts a string, and props may be `undefined`.
- In `log-internal.js`, I had to remove the `raiseError()` function because TypeScript is unable to understand that a function, when called, will always throw. The fix was just to replace with `throw new Error...`
- Introduced a var in `mjpeg.js` to avoid repeated casting
- Added some type guards to `net.js`. as function parameters, union types kind of suck. This is definitely one place TS would have helped, since it allows function overloads. Also dropped use of deprecated `url` module in lieu of W3C `URL`.
- Removed some old-node-version cruft from `util.js`.
Non-code changes:
- Needed to use an override for `type-fest`, as it's included as a transitive dependency, but otherwise is only present in the devdeps of the monorepo root `package.json`. This means, due to version conflicts, a `support/node_modules/type-fest` would be created with an old, incompatible version, which would be preferred over the monorepo one due to Node's resolution algorithm. This needs npm v8.3+ to work properly, but is only a problem for contributors (not consumers).
- Pinned type of `opencvv-bindings`.
- No longer deleting `build` dir before transpilation since it may contain generated declarations!
This changes the TS configuration to to a) emit declarations for supported packages, and b) build declarations incrementally by package.
To begin, we are targeting `appium`, `@appium/base-driver` and `@appium/support` for declarations; these three are intended to be published with declarations generated via their JS.
Both `@appium/support` and `appium` are fully typechecked (sans implicit `any` types), but `@appium/base-driver` is mostly not. Regardless, declarations are generated for all three.
Each package will have its own `tsconfig.json` which "inherits" from `config/tsconfig.base.json`. Each will also need to declare relative paths to dependencies. e.g., the `appium` package must configure its `tsconfig.json` so that it depends on the declarations of `@appium/support` and also where to find `@appium/support`. Essentially, a small dependency tree gets built, and those at the "root" get their declarations built first; then the consumers use those declarations.
Also reorganized some scripts and added `npm-run-all` to help and added a "typecheck" job to CI.
Since `npm install` will build, it's pretty inconvenient to have `npm` abort installation if the build fails. In that case, if any of the build steps fail (see `build:loose`) the installation will continue. Added a `prepublishOnly` which runs a build in "strict" mode where everything must pass.
_Note: there's still a strong coupling of "what `BaseDriver` provides" and "what an external driver *may* provide". It's unclear to me if this is a problem, though there seem to be a handful of methods that external drivers *must* implement. The easiest way to get around this would probably be to extract these required properties and methods into their own interface (duplication) if we cannot otherwise declare methods as "abstract". As it stands, the `Driver` is basically just `BaseDriver`._
Also:
- Moved some configuration types out of `appium` and into here, since they are used both by `appium` and `base-driver` via `DriverOpts`
- Split `Driver` type into "the thing that BaseDriver is" and "all the other methods external drivers can implement"
- Added missing `proxyCommand` method
- Better generics for `LogType`
- Remove unused `Constructor` type
- Better types for `getSession`/`getSessions`
- Added util `Class` type
- Added many missing typings from DefinitelyTyped
- Added `ts-node` for `@wdio/types` (I need to send a PR to webdriver to eliminate this dependency)
- Type checks in their CI step
This PR removes Gulp from `@appium/base-driver` and reorganizes the test files.
The build needs to be a little different since other modules depend on its `build/test/basedriver` module. IMO this should probably get moved elsewhere, but this does not break compatibility.
This PR removes Gulp from `@appium/fake-driver` as already done in a couple other packages.
Also: moved `screen.png` into `lib/` since `@babel/cli` can just copy it over into `build/`.
Christopher Hiller