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

webdriverio: Update types for attach and remote methods #3953

Conversation

MunGell
Copy link
Contributor

@MunGell MunGell commented May 9, 2019

Proposed changes

This is a proposal to address some part of #3942

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have read the CONTRIBUTING doc
  • N/A I have added tests that prove my fix is effective or that my feature works
  • N/A I have added necessary documentation (if appropriate)

Further comments

This PR contains solution where we suggest passing merged options (WebdriverIO and WebDriver) to the remote() function, since this is what code is expecting on the input.

The other option would be to re-engineer the way we work with options, but this most probably will lead to a breaking change in public API.

Reviewers: @webdriverio/technical-committee

@codecov
Copy link

codecov bot commented May 9, 2019

Codecov Report

Merging #3953 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3953   +/-   ##
=======================================
  Coverage   98.55%   98.55%           
=======================================
  Files         153      153           
  Lines        3451     3451           
  Branches      753      753           
=======================================
  Hits         3401     3401           
  Misses         45       45           
  Partials        5        5

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8de7f1a...103c36f. Read the comment docs.

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@christian-bromann christian-bromann added the PR: Docs 📝 PRs that contain changes to the documentation label May 9, 2019
@christian-bromann christian-bromann merged commit ff215b8 into webdriverio:master May 9, 2019
@ost12666
Copy link

Is this version released? When I install the last version with npm I still get the error here:
options?: WebDriver.AttachSessionOptions

@christian-bromann
Copy link
Member

@ost12666 this should be implemented

@ost12666
Copy link

I saw the PR but when I do npm install webdriverio I get 5.8.4 with the problem. Any idea?

node_modules/webdriverio/webdriverio.d.ts:41:29 - error TS2694: Namespace 'WebDriver' has no exported member 'AttachSessionOptions'.

41 options?: WebDriver.AttachSessionOptions,
~~~~~~~~~~~~~~~~~~~~

@mgrybyk
Copy link
Member

mgrybyk commented May 16, 2019

@christian-bromann @ost12666 in order this one to be fixed webdriver package has to be released as well.
We have released webdriverio package only.

@ost12666
Copy link

So any suggestion what to do for now? I tried to set noEmitOnError to false in tsconfig but it still stops with error.

@christian-bromann
Copy link
Member

We have released webdriverio package only.

Yeah I think every change to the TS definition should come with a change of the definition inside the package (adding a comment or anything) so that Lerna detects a change there as well and releases it. I will make sure to point this out moving forward.

@ost12666
Copy link

thanks! any idea for a work around now since I am kind of stuck...

@mgrybyk
Copy link
Member

mgrybyk commented May 16, 2019

Define your own typings, similar to how it is in webdriver package.
And remove workaround when new version of webdriver package is released

@ost12666
Copy link

thanks I guess that can work

@ost12666
Copy link

Could not do that since I am not using WebDriver directly but this does the trick in tsconfig.json

"skipLibCheck": true,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Docs 📝 PRs that contain changes to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants