feat: add all the types #819

boneskull · 2023-06-14T23:21:26Z

this PR requires both appium/appium-adb#661 and appium/appium#18721 to be published for the build to pass.

note: I did not type the entire codebase, but all of the commands are typed, as is the driver proper.

moves all @typedefs which aren't aliases into types.ts and created many more
each file now contains a single object literal, which is the mixin
all methods in the mixin are defined in the object literal. this allows us to use ThisType to avoid needing to use @this everywhere
most everything remains js except the "find" mixin
the mixin function and all mixin interfaces are defined in mixins.ts, in addition to the declaration merge with AndroidDriver
the coverage mixin was removed entirely per a slack convo
add missing appWaitForLaunch constraint
update ESLint
upgrade TS, use strict types

boneskull · 2023-06-14T23:21:41Z

Current dependencies on/for this PR:

master
- PR chore: build with typescript instead of babel #813
  - PR chore: add prettier, commitlint, swap husky #814
    - PR feat: add all the types #819 👈

This comment was auto-generated by Graphite.

jlipps · 2023-06-14T23:23:01Z

looks like appium-adb is published, and i'll run another publish of appium soon

boneskull · 2023-06-14T23:27:00Z

@jlipps hold off on publishing appium; want to make sure xcuitest works ok with those changes

boneskull · 2023-06-15T00:36:31Z

@jlipps actually, xcuitest is managed by renovate right, so I think it's okay to publish. was just worried loose deps would sink the ship

mykola-mokhnach · 2023-06-15T06:01:03Z

index.ts

@@ -0,0 +1,28 @@
+import {install} from 'source-map-support';


Didn't we already have the same issue in appium/node-teen_process#253 ?

@jlipps could you please explain better what the actual issue was?

@mykola-mokhnach this should be ok. it's a typescript file and is transpiled. so long as package.json's main field points to the transpiled code this will be ok.

as far as I could tell the problem there was with tsconfig.json, not source-map-support

mykola-mokhnach · 2023-06-15T06:02:50Z

index.ts

+} from './lib/helpers/webview';
+
+/**
+ * @privateRemarks `androidCommands` was previously exported as an object


we only need mixin commands there. Methods defined in the driver class itself must be ignored

because AndroidDriver class itself contains stuff which is specific for the obsolete driver. It must not be used neither in uia2 nor in espresso

ok, I changed this to the way it was

but also, ideally, uia2 and espresso would be subclasses of AndroidDriver, which renders it moot; they will get everything in AndroidDriver and would need to override any prop on the prototype they don't want

right now it is a collection of various mixins and helper methods which matter in this component.
UIA2 and Espresso are too different to prefer subsclassing over composition.

lib/commands/app-management.js

lib/commands/context.js

lib/commands/mixins.ts

jlipps

this PR is impossible to really review. I think we should probably just merge/publish as a beta and then do a bunch of testing with uiauto2/espresso

jlipps · 2023-06-14T23:35:53Z

index.ts

+ * prototype from `AndroidDriver` proper.  Hopefully, doing it this way won't
+ * break anything.
+ */
+export const androidCommands = {...AndroidDriver.prototype};


i think the rationale was that these commands were exported so they could be applied to the prototype of the subclass. it was meant to be "the set of driver commands", not "the set of everything on the AndroidDriver prototype"

so yeah i don't know if this is a bad idea or not. maybe we don't really need this anymore if the commands are applied to the AndroidDriver prototype and all the subclasses extend AndroidDriver?

I guess it remains to be seen. I really have no idea why this is necessary at all

lib/commands/actions.js

boneskull · 2023-06-15T19:24:34Z

@jlipps that sounds good. this will automatically be published if we merge it, so... maybe we should just publish it from the head branch?

boneskull · 2023-06-15T19:25:21Z

@mykola-mokhnach why does the PR title have to be in conventional commit format? does it somehow affect the release or changelog?

mykola-mokhnach · 2023-06-15T19:34:47Z

@mykola-mokhnach why does the PR title have to be in conventional commit format? does it somehow affect the release or changelog?

yes, it affects the changelog and also the logic for automated versions bumping

jlipps · 2023-06-15T19:34:59Z

@jlipps that sounds good. this will automatically be published if we merge it, so... maybe we should just publish it from the head branch?

yeah I think so, maybe just publish a beta from this PR branch and we can work with it for a while. this repo doesn't see many commits so i don't anticipate much divergence. we could just freeze all work not related to this PR while we're working on it.

boneskull · 2023-06-16T18:49:14Z

OK, the build is looking good, so I'm going to publish this as a beta or something.

boneskull · 2023-06-29T19:10:29Z

as of right now, this needs new stuff in @appium/types to pass a build

jlipps · 2023-06-29T20:10:35Z

does the release of appium I pushed a few hours ago do the trick?

boneskull · 2023-06-29T20:57:43Z

@jlipps no it needs appium/appium#18817

boneskull · 2023-07-03T20:33:12Z

assuming this build passes, I'll release another alpha off of it and deal w/ appium/appium-uiautomator2-driver#635

jlipps · 2023-07-04T20:59:44Z

👍

- upgrade typescript - actually ship types - add missing `type-fest` dep chore: use strict types where enabled chore: update eslint config chore: add missing `appWaitForLaunch` constraint; rename file chore: reorganize helpers chore: remove appium-adb stub feat: add types to all commands chore: review changes chore: add wallaby config chore: fix all the tests chore(deps): upgrade to latests 5.14.0-alpha.0 fix: add missing files to release 5.14.0-alpha.1 chore: move index.ts into lib fix: broken types fix(types): fix unknown-related types fix(types): allow generic settings, createresult fix: fix broken main field fix: upgrade appium-chromedriver fix(types): fix inconsistent signature of execute() chore: upgrade appium deps 5.14.0-alpha2 fix: remove unused endCoverage cmd 5.14.0-alpha.3

This was referenced Jun 14, 2023

chore: build with typescript instead of babel #813

Merged

chore: add prettier, commitlint, swap husky #814

Merged

boneskull marked this pull request as draft June 14, 2023 23:23

boneskull self-assigned this Jun 14, 2023

mykola-mokhnach reviewed Jun 15, 2023

View reviewed changes

lib/commands/app-management.js Show resolved Hide resolved

mykola-mokhnach reviewed Jun 15, 2023

View reviewed changes

lib/commands/context.js Show resolved Hide resolved

mykola-mokhnach reviewed Jun 15, 2023

View reviewed changes

lib/commands/mixins.ts Show resolved Hide resolved

jlipps reviewed Jun 15, 2023

View reviewed changes

boneskull force-pushed the boneskull/actual-mixins branch 2 times, most recently from 6aefcdf to 453d2dc Compare June 15, 2023 21:18

boneskull changed the title ~~ok here is the huge PR with types~~ feat: add all the types Jun 15, 2023

boneskull force-pushed the boneskull/actual-mixins branch from 53358c2 to 77f29db Compare June 15, 2023 21:37

boneskull marked this pull request as ready for review June 15, 2023 23:02

Base automatically changed from boneskull/mixins to master June 15, 2023 23:02

boneskull force-pushed the boneskull/actual-mixins branch 2 times, most recently from db5c19b to e677125 Compare June 16, 2023 18:45

boneskull force-pushed the boneskull/actual-mixins branch from 14e0b07 to ccf99ba Compare June 29, 2023 19:05

boneskull mentioned this pull request Jun 30, 2023

feat: add all the types appium/appium-uiautomator2-driver#635

Merged

jlipps mentioned this pull request Aug 14, 2023

need to update remote debugger selenium atoms to latest appium/appium#18951

Closed

jlipps force-pushed the boneskull/actual-mixins branch from d62fc48 to e3e95af Compare August 28, 2023 21:39

jlipps force-pushed the boneskull/actual-mixins branch from e3e95af to a790400 Compare August 28, 2023 21:46

jlipps merged commit ceb852d into master Aug 28, 2023

jlipps deleted the boneskull/actual-mixins branch August 28, 2023 22:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add all the types #819

feat: add all the types #819

boneskull commented Jun 14, 2023

boneskull commented Jun 14, 2023

jlipps commented Jun 14, 2023

boneskull commented Jun 14, 2023

boneskull commented Jun 15, 2023

mykola-mokhnach Jun 15, 2023

boneskull Jun 15, 2023

mykola-mokhnach Jun 15, 2023

jlipps Jun 15, 2023

boneskull Jun 15, 2023

mykola-mokhnach Jun 15, 2023

boneskull Jun 15, 2023

mykola-mokhnach Jun 15, 2023

boneskull Jun 15, 2023

boneskull Jun 15, 2023

mykola-mokhnach Jun 16, 2023

jlipps left a comment

jlipps Jun 14, 2023

jlipps Jun 14, 2023

boneskull Jun 15, 2023

boneskull commented Jun 15, 2023

boneskull commented Jun 15, 2023

mykola-mokhnach commented Jun 15, 2023

jlipps commented Jun 15, 2023

boneskull commented Jun 16, 2023

boneskull commented Jun 29, 2023

jlipps commented Jun 29, 2023

boneskull commented Jun 29, 2023

boneskull commented Jul 3, 2023

jlipps commented Jul 4, 2023

feat: add all the types #819

feat: add all the types #819

Conversation

boneskull commented Jun 14, 2023

boneskull commented Jun 14, 2023

jlipps commented Jun 14, 2023

boneskull commented Jun 14, 2023

boneskull commented Jun 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlipps left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boneskull commented Jun 15, 2023

boneskull commented Jun 15, 2023

mykola-mokhnach commented Jun 15, 2023

jlipps commented Jun 15, 2023

boneskull commented Jun 16, 2023

boneskull commented Jun 29, 2023

jlipps commented Jun 29, 2023

boneskull commented Jun 29, 2023

boneskull commented Jul 3, 2023

jlipps commented Jul 4, 2023