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

Add Interfaces for each domain type used #43

Closed
ebebbington opened this issue Jan 17, 2021 · 1 comment · Fixed by #45
Closed

Add Interfaces for each domain type used #43

ebebbington opened this issue Jan 17, 2021 · 1 comment · Fixed by #45
Labels
Type: Chore Merging this pull request results in a patch version increment

Comments

@ebebbington
Copy link
Member

Summary

What:

Add an interface for each sub domain type used, so this can be used in it's respective method

Why:

So we can better type the return values from sendWebSocketMessage, it'll give us a clearer idea on what will be returned for that respective method

How:

namespace Page {
  export interface createIsolatedWorld {
     executionContentId: number
  }
}

public evaluatePage(...) {
  ...
   const result = await this.sendWebSocketMessage(
        "Page.createIsolatedWorld",
        {
          frameId: this.frame_id,
        },
      ) as Page.createIsolatedWorld;
}

These must also account ERRORS (eg frameId is "blahblahblah")

@ebebbington ebebbington added Type: Chore Merging this pull request results in a patch version increment Priority: Low labels Jan 17, 2021
@ebebbington
Copy link
Member Author

Noticing there's theres a tiny problem here, i was hoping we could do something like:

import { Runtime } from "./interfaces/domains/runtime.ts"
public assertSee(...) {
  const res = await this.sendWebSocketMessage("Runtime.evaluate", {
      expression: command,
    }) as Runtime.evaluate
  ...
}

But the expression param we pass in for Runtime.evaluate will change the response object based on the value:

expression: document.body.indexOf(text)  > -1 // { result: { type: "boolean", value: false } }
expression: console.log(a)
{
  result: {
    type: "object",
    subtype: "error",
    className: "ReferenceError",
    description: "ReferenceError: a is not defined\n    at <anonymous>:1:13",
    objectId: '{"injectedScriptId":2,"id":1}'
  },
  exceptionDetails: {
    exceptionId: 1,
    text: "Uncaught",
    lineNumber: 0,
    columnNumber: 12,
    scriptId: "12",
    exception: {
      type: "object",
      subtype: "error",
      className: "ReferenceError",
      description: "ReferenceError: a is not defined\n    at <anonymous>:1:13",
      objectId: '{"injectedScriptId":2,"id":2}'
    }
  }
}

And. in the assertSee method, the res is always going to be { result: { type: "boolean", value: false } } because we. are specifically using. document.bodyd.indexOf..., so i don't. think it would make too much sense to add interfaces for each domain type used, for example, if we did use Rumtime.evaluate, we'd have. to do:

const res = ... as Runtime.evaluate(...)
if (res.result.value) {
   // ...
}

It's making us use the conditional (as value isn't always present on res.result), but it is when our expression is document.body.indexOf(...)

This leads me to think, we should scrap creating an interface for each domain, and check the result value in each method, and add the type directly, eg:

public assertSee(...) {
  const res = ... as {
    result: {
      type: "boolean",
      value: boolean
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Chore Merging this pull request results in a patch version increment
Development

Successfully merging a pull request may close this issue.

1 participant