Move some type definitions to Flow #1335
Conversation
There was a problem hiding this comment.
I paired with @ppold on this. Couple comments:
- we couldn't find a nice way to share the types between the client-adapter and debugger so they're currently duped
- we couldn't get the client-adapter to fail. This is probably the problem with 1 as well :). We checked the .flowconfig and it looks like it shouldn't be ignored...
| * @static | ||
| */ | ||
| export type BreakpointResult = { | ||
| ids: string, |
There was a problem hiding this comment.
NOTE: we tried to make flow fail by adding this error, but when we run flow we got no errors
There was a problem hiding this comment.
That's a blocker for this PR. :) We can't merge this until we know it's actually working. If it's not applying the types we have no idea if this PR actually works.
There was a problem hiding this comment.
💯 sorry that wasn't clear.
| @@ -1,6 +1,96 @@ | |||
| // @flow | |||
|
|
|||
| /** | |||
| * Source File Location | |||
There was a problem hiding this comment.
we tried doing something like this:
import type { Breakpoint, Source } from "devtools-client-adapters/types";
export type { Breakpoint, Source };This worked to the extent that we didn't have to copy the types and the debugger would mention when a type was missing, but it didn't help us w/r/t to validating the types.
|
ping @ppold will you be able to look at this? |
| @@ -15,7 +15,6 @@ import type { List } from "immutable"; | |||
| import type { Frame } from "../types"; | |||
|
|
|||
| if (typeof window == "object") { | |||
There was a problem hiding this comment.
This whole block should be removed not left empty.
|
Hmm, things have moved a bit:
|
|
closing this as most of the work will be done in devtools-core |
Summary of Changes
Test Plan
Make the types pass with Flow