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

added experimental typescript support #1125

Merged

Conversation

nuclearglow
Copy link
Contributor

@nuclearglow nuclearglow commented Mar 12, 2020

Closes #209

  • updated docs
  • exposed typings through package.json
  • Bonus: added package.json schema

This solution would mean that everyone using Taiko with TypeScript would be exposed to the Typings provided, but there is also an optional solution which would make the typings optional but requires a bit of configuration work for every project using it:

  • the types folder in the repo would contain a taiko folder with a index.d.ts
  • the typings could be consumed using the "types" field in tsconfig.json
  • a user with gauge-ts setup would have to edit tsconfig.json in this way:
{
    "compilerOptions": {
        "typeRoots": ["node_modules/@types", "node_modules/taiko/types"],  // add available taiko TypeScript Type Definition folder to the project
        "types": ["node", "taiko"] // use taiko types in the project
    }
}

If you prefer to keep this optional, let me know and I will update the PR.
Possible TODOs:

  • The typings themselves are workable, but likely need more work to reflect the current API better.
  • The API reflected is 1.0.2, I did not include any changes from there to the current version

Hopefully, we can work together to include typings for Taiko :-)

@gaugebot gaugebot bot added the cla-signed label Mar 12, 2020
@gaugebot gaugebot bot requested a review from negiDharmendra March 12, 2020 09:36
@nuclearglow
Copy link
Contributor Author

Here is an example repo I just created using this PR branch for a Gauge TS Taiko example:
https://github.com/nuclearglow/gauge-ts-taiko-types-example

sriv
sriv previously approved these changes Mar 16, 2020
@sriv sriv dismissed their stale review March 16, 2020 02:36

need to discuss reflect mechanism

@sriv
Copy link
Member

sriv commented Mar 16, 2020

@nuclearglow - what do you suggest is the best way to keep the types in sync? Is there any automated mechanism that you know of, which can be hooked up t0 husky?

Do you think it makes sense to port some tests to TS and make them run in the pipeline?

thanks for the contribution!

PS - i had approved this PR, but then I figured we should discuss these...

@nuclearglow
Copy link
Contributor Author

@sriv yes, I had a look at available tools that produce typings automatically, but my preliminary assessment is that they do not really work with the Taiko API, as it is really flexible and allows a lot in the method parameters, all of this is pretty hard to configure out automatically. So I would recommend that extensive testing of the typings should be implemented. It would certainly mean that a TypeScript project needs to test the Taiko API methods using the available parameters and should check whether the project still compiles. This way, any future changes to the API which breaks the typings would mean that TypeScript tests are failing - and then the typings could be enhanced to reflect the newest API changes. Of course, new methods would need new tests and possibly new typings.

@nuclearglow
Copy link
Contributor Author

I would also recommend that Taiko declares TypeScript support as experimental for the next releases until we work these things out and would therefore only allow projects to set up the typings manually, this way we can get some feedback from real world projects out there if the typings work against their test projects - and at the same time we do not break existing TypeScript test projects where the typings are inadequate.

If you think that is the way to go, I will update this PR so that the README, the folder structure and the setup only supports manually configured projects, just let me know.

@nuclearglow
Copy link
Contributor Author

The most important thing we should discuss within the Typings themselves is the TaikoSearchElement (https://github.com/getgauge/taiko/pull/1125/files#diff-6d9454e03789edeaeb913b3ff375045fR64), as you can see this basically allows everything and it shows a weak point in my typings. But I do not know enough about how the Taiko API works internally to provide stronger typings or a better interface...

@sriv
Copy link
Member

sriv commented Mar 16, 2020

So I would recommend that extensive testing of the typings should be implemented.

Agreed.

I would also recommend that Taiko declares TypeScript support as experimental for the next releases until we work these things out

Fair enough. We usually toggle experimental features off, but manual config to enable it works equally well.

On the types -

  • is there a reason to have Taiko prefixed on most (all) types? Would it be possible to use namespaces instead?

Regarding TaikoSearchElement - in taiko, the input to any element search can be either -

  • string
  • instance of elementwrapper
  • instance of element

As a result you'll see checks like this: https://github.com/getgauge/taiko/blob/master/lib/taiko.js#L1151

it'd be nice to have types, but we are just touching the interface here. I think it'd be good to constrain TaikoSeachElement to just these. Thoughts?

@negiDharmendra negiDharmendra requested review from sriv and removed request for negiDharmendra March 17, 2020 07:38
@nuclearglow
Copy link
Contributor Author

Hi @sriv thanks for your feedback, I have worked on the PR and made some updates:

  • experimental TypeScript support now needs to be configured manually in the gauge-ts project's tsconfig.json, I have updated the README with an example. Also see my updated example repo which is configured to work with the PR branch -> https://github.com/nuclearglow/gauge-ts-taiko-types-example
  • the taiko/index.d.ts already declare a module taiko, so I have removed the Taiko prefix everywhere, should work fine, if there are name clashes, usually VS Code or IntelliJ allows to choose from the module, which would then be taiko for us. An import in StepImplementation.ts would look like this: import { BrowserOptions } from 'taiko';
  • I have not changed the TaikoSearchElement right now. I have taken a deeper look at this and I want to discuss an example from your API: https://docs.taiko.dev/#click. Here, it is possible to provide a string, a selector and an Object, such as {x: int, y: int}. I have not typed this, instead I allow generic objects. Either we provide interfaces to deal with Objects in the taiko API or we use the generic option. I would like the former, but I do not know how much work that would be. What do you think? Should we implement specific interfaces for the Object parameters? I might need help to catch them all...

@sriv
Copy link
Member

sriv commented Mar 23, 2020

👍 on (1) and (2).

Regarding TaikoSearchElement: I too like the first option of having interfaces rather than generics.

string is straightforward. For selector and Object, taiko looks for Object to map to an Element. Since JS does not make type safety easy, we added a helper method IsSelector and IsElement.

Does this help? Perhaps we can start porting some of our tests to use the TS bindings?

@nuclearglow
Copy link
Contributor Author

nuclearglow commented Apr 3, 2020

Hey @sriv thanks for the pointers to the helper methods, I managed to learn a bit more. I think we can restrict the SearchElement type to the types you mentioned, but I will need to go through the API and be on the lookout for special cases. I found this in taiko.js:1300:

 if (isSelector(selector) || isString(selector) || isElement(selector)) {
...
  } else {
    options = setClickOptions(options, selector.x, selector.y);

This basically means that there are special cases for the selector which are going further than selector, string and element types. So I propose that we restrict the SearchElement to the instances you mentioned, I will analyse the helper methods to see that I catch everything you check. And for special parameters like in click() where a coordinate can be given as a SearchElement, I would then type something like this:

selector: SearchElement | MouseCoordinates

And yes, we could start porting some test cases to use TypeScript, can you help with the initial setup and maybe provide an example if you know to to go about this, then I could help with some tests as far as my project budget allows. If we should think about how to do it, let me know, I will help.

@nuclearglow
Copy link
Contributor Author

nuclearglow commented Apr 3, 2020

OK, a new update with some updated typings after I have taken a look at the helper functions and the API again. Basically, I have upgraded the basic SerachElement definition to allow only the stuff that Taiko would accept:

    // BasicSelector mimics isSelector
    export interface BasicSelector {
        // FIXME: @sriv: is that correct?
        elements: Element[] | Node[] | string[];
        exists(): boolean;
        [key: string]: any;
    };
    // isSelector also allows ElementWrapper instances
    export type Selector = BasicSelector | ElementWrapper;
    // SearchElement mimics isSelector, isString, isElement
    export type SearchElement = string | Selector | Element;

@nuclearglow nuclearglow force-pushed the feature/209-typescript-bindings branch from 802ade8 to c37361f Compare April 3, 2020 14:24
@sriv
Copy link
Member

sriv commented Apr 6, 2020

Sounds like good progress @nuclearglow ! I will take a closer look soon (feel free to ping me here if I am blocking your work), in the meantime there are some merge conflicts :)

@gaugebot
Copy link

gaugebot bot commented Apr 15, 2020

@nuclearglow Thank you for contributing to taiko. Your pull request has been labeled as a release candidate 🎉🎉.

Merging this PR will trigger a release.

Please bump up the version as part of this PR.

Instructions to bump the version can found at CONTRIBUTING.md

If the CONTRIBUTING.md file does not exist or does not include instructions about bumping up the version, please looks previous commits in git history to see what changes need to be done.

types/taiko/index.d.ts Show resolved Hide resolved
// https://taiko.gauge.org/#title
export function title(): Promise<string>;
// https://taiko.gauge.org/#click
export function click(selector: SearchElement | MouseCoordinates, options?: ClickOptions, args?: string[]): Promise<void>;
Copy link
Member

Choose a reason for hiding this comment

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

args is a rest parameter and its type has to be RelativeSearchElement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

// https://taiko.gauge.org/#click
export function click(selector: SearchElement | MouseCoordinates, options?: ClickOptions, args?: string[]): Promise<void>;
// https://taiko.gauge.org/#doubleclick
export function doubleClick(selector: SearchElement | MouseCoordinates, options?: BasicNavigationOptions, args?: string[]): Promise<void>;
Copy link
Member

Choose a reason for hiding this comment

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

args is a rest parameter and its type has to be RelativeSearchElement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

types/taiko/index.d.ts Outdated Show resolved Hide resolved
types/taiko/index.d.ts Outdated Show resolved Hide resolved
types/taiko/index.d.ts Outdated Show resolved Hide resolved
types/taiko/index.d.ts Outdated Show resolved Hide resolved
types/taiko/index.d.ts Show resolved Hide resolved
types/taiko/index.d.ts Show resolved Hide resolved
types/taiko/index.d.ts Outdated Show resolved Hide resolved
zabil and others added 6 commits April 17, 2020 14:47
* Add FAQ file with general section

* Remove summary text

* Use summary tag for questions

* Add FAQ for firefox support and installation

* Add more FAQ and move to docs folder

* Add link to FAQ in the TOC

* Add link to Typescript and .npmrc info

Co-authored-by: Srikanth <srikanth.ddit@gmail.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
…through package.json, added package.json schema

Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
…ge.json, updated README

Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
…e taiko already

Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Sven Vowé and others added 20 commits April 17, 2020 14:52
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
* getgauge#818 Fix validation for browser session

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#818 fix closeWindow

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#818  Update validation error message

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#818 update error messaging

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

Co-authored-by: Srikanth <srikanth.ddit@gmail.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Bumps [prettier](https://github.com/prettier/prettier) from 2.0.3 to 2.0.4.
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/master/CHANGELOG.md)
- [Commits](prettier/prettier@2.0.3...2.0.4)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Fixes getgauge#1175

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Bumps [documentation](https://github.com/documentationjs/documentation) from 12.2.0 to 12.3.0.
- [Release notes](https://github.com/documentationjs/documentation/releases)
- [Changelog](https://github.com/documentationjs/documentation/blob/master/CHANGELOG.md)
- [Commits](documentationjs/documentation@v12.2.0...v12.3.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
* [WIP] getgauge#1134 Case insensitive selectors for radioButton

* [WIP] getgauge#1134 Case insensitive selectors for TextBox

* [WIP] getgauge#1134 Case insensitive selectors for checkbox

* remove permissions map, allow users to use CDP Permissions

Signed-off-by: sriv <srikanth.ddit@gmail.com>

Co-authored-by: Gabriel Peixinho <gabriel.peixinho@gmail.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Bumps [sinon](https://github.com/sinonjs/sinon) from 9.0.1 to 9.0.2.
- [Release notes](https://github.com/sinonjs/sinon/releases)
- [Changelog](https://github.com/sinonjs/sinon/blob/master/CHANGELOG.md)
- [Commits](https://github.com/sinonjs/sinon/commits)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
* getgauge#612 add time field

selector that supports input type date, datetime-local, month, time, week

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* Bump lint-staged from 10.0.10 to 10.1.0

Bumps [lint-staged](https://github.com/okonet/lint-staged) from 10.0.10 to 10.1.0.
- [Release notes](https://github.com/okonet/lint-staged/releases)
- [Commits](lint-staged/lint-staged@v10.0.10...v10.1.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* Bump documentation from 12.1.4 to 12.2.0

Bumps [documentation](https://github.com/documentationjs/documentation) from 12.1.4 to 12.2.0.
- [Release notes](https://github.com/documentationjs/documentation/releases)
- [Changelog](https://github.com/documentationjs/documentation/blob/master/CHANGELOG.md)
- [Commits](documentationjs/documentation@v12.1.4...v12.2.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#612  add tests and update timeField

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* Bump lint-staged from 10.1.0 to 10.1.1

Bumps [lint-staged](https://github.com/okonet/lint-staged) from 10.1.0 to 10.1.1.
- [Release notes](https://github.com/okonet/lint-staged/releases)
- [Commits](lint-staged/lint-staged@v10.1.0...v10.1.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* Fix beforeunload API (getgauge#1157)

* Fixed beforeunload API. getgauge#1093

       - Fixed existing API contract to only accept callback.
       - Wait for beforeunload popup handler to finish before closing the browser.

* Fixed javascript popup FTs. getgauge#1093

* Force close browser after 5 second. getgauge#1093

* Fixed typo in warning message

* Update beforeunload API documentation getgauge#1093

Co-authored-by: Srikanth <srikanth.ddit@gmail.com>
Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* Make coresponding wrappers to use value wrapper

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#612 update timeField to use date object to select

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#612 wait for navigation and highlight element on action

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#612 add tests and fixes

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* Update timeField.js

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#612 fix min max validation and add tests

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#612 add more tests and fix min max validation

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#612  update docs

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* getgauge#612 Fix docs and test description

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Dharmendra Singh <dharmenn@thoughtworks.com>
Co-authored-by: Srikanth <srikanth.ddit@gmail.com>
Co-authored-by: Zabil Cheriya Maliackal <zabil@users.noreply.github.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
This is the fix of npm audit fix command

Signed-off-by: Zabil Cheriya Maliackal <zabilcm@gmail.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Bumps [husky](https://github.com/typicode/husky) from 4.2.3 to 4.2.5.
- [Release notes](https://github.com/typicode/husky/releases)
- [Changelog](https://github.com/typicode/husky/blob/master/CHANGELOG.md)
- [Commits](typicode/husky@v4.2.3...v4.2.5)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Bumps [lint-staged](https://github.com/okonet/lint-staged) from 10.1.2 to 10.1.3.
- [Release notes](https://github.com/okonet/lint-staged/releases)
- [Commits](lint-staged/lint-staged@v10.1.2...v10.1.3)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
* fix element screenshot with proximity selectors

use isElement instead of Object

Signed-off-by: Sai <saikrishna321@yahoo.com>

* update patch version

Signed-off-by: saikrishna321 <saikrishna321@yahoo.com>

Co-authored-by: Dharmendra Singh <dharmenn@thoughtworks.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Bumps [eslint-plugin-prettier](https://github.com/prettier/eslint-plugin-prettier) from 3.1.2 to 3.1.3.
- [Release notes](https://github.com/prettier/eslint-plugin-prettier/releases)
- [Changelog](https://github.com/prettier/eslint-plugin-prettier/blob/master/CHANGELOG.md)
- [Commits](https://github.com/prettier/eslint-plugin-prettier/commits)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
* fix screenshot for specific element with index

Signed-off-by: saikrishna321 <saikrishna321@yahoo.com>

* use isSelector fn from helpers

Signed-off-by: saikrishna321 <saikrishna321@yahoo.com>

Co-authored-by: Dharmendra Singh <dharmenn@thoughtworks.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
* Update chromium to revision 756035

* Add browser dependencies

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* Install dependencies only for linux

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

* Install browser dependencies for FT

Signed-off-by: NivedhaSenthil <nivedhasenthil@gmail.com>

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: NivedhaSenthil <nivedhasenthil@gmail.com>
Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
@nuclearglow nuclearglow force-pushed the feature/209-typescript-bindings branch from 7b56f15 to f4b873b Compare April 17, 2020 12:55
@nuclearglow
Copy link
Contributor Author

nuclearglow commented Apr 17, 2020

Hi @NivedhaSenthil thank you for the great feedback, I have tried to add everything as far as I understood it. I still have some trouble with the relative searches, they are hard to type since it is a bunch of methods... But I superficially typed the RelativeSearchElement and added it as the return type of the proximity methods, I hope this will work out. At this point, I would need real TS tests to check that everything can be typed as we envision it. I will try to add some to my gauge TS example project soon. Please review my changes and let me know if you have more feedback!

Signed-off-by: Sven Vowé <sven.vowe.ext@dwpbank.de>
@NivedhaSenthil
Copy link
Member

PR looks fine for an initial TS support, this is a great effort :) Will try adding some test in TS and port some existing test also.. Do follow #1152 for any progress.

@NivedhaSenthil NivedhaSenthil merged commit 00586ec into getgauge:master Apr 20, 2020
@ghost
Copy link

ghost commented Jul 10, 2020

Hi there. If I understand this thread correctly then the experimental Typescript is only available if Taiko is used in conjunction with Gauge. Is this correct? If not, could someone maybe explain (to a non-expert) how Typescript could be used for writing test cases if other test frameworks (e.g. Jest, Mocha) are used?

@zabil
Copy link
Member

zabil commented Jul 10, 2020

Typescript is only available if Taiko is used in conjunction with Gauge.

The typescript bindings are for Taiko. You can use these binding with any test runner. You can refer https://docs.taiko.dev/experimental_features/#typescript-support on adding configuration to tsconfig.json

@ghost
Copy link

ghost commented Jul 10, 2020

@zabil Great, that has clarified it, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Add typescript bindings
10 participants