chore: Upgrade TypeScript to 3.6 #5559
Conversation
|
@TimvdLippe happy to take any thoughts you have on TypeScript and if there's better workarounds. My approach here is I'd rather do the minimum reasonable fixes to get TS happy so we can get up to 3.8, and then solve these problems "properly" when we migrate the source code to |
jackfranklin
force-pushed
the
upgrade-typescript-3.6
branch
from
8492ec5
to
5b2b9a4
Compare
There was a problem hiding this comment.
LGTM but I'd like for @TimvdLippe to take a look as well before landing.
| @@ -26,7 +26,7 @@ const {Keyboard, Mouse, Touchscreen} = require('./Input'); | |||
| const Tracing = require('./Tracing'); | |||
| const {helper, debugError, assert} = require('./helper'); | |||
| const {Coverage} = require('./Coverage'); | |||
| const {Worker} = require('./Worker'); | |||
| const {Worker: PuppeteerWorker} = require('./Worker'); | |||
There was a problem hiding this comment.
@mathiasbynens Can the export of Worker be renamed to PuppeteerWorker in Worker.js or does that constitute a breaking change?
There was a problem hiding this comment.
I think that would be a breaking change if we support people who have done:
const { Worker } = require('puppeteer/lib/Worker')I'm not sure how common that is - @mathiasbynens WDYT?
There was a problem hiding this comment.
It's a potentially breaking change that we can avoid in this case, so let's just stick to the more conservative change for now.
Continues the work to get up to TS 3.8 (latest release at time of writing). This version of TS introduced built in definitions for web workers that include an `interface Worker` so TS gets confused when it sees us reference a `Worker`. I have renamed the imports to `PuppeteerWorker` as I couldn't figure out a way to tell TS to not load in the worker types; longer term we might consider renaming `Worker` to `PuppeteerWorker` (or an alternative) but that would be a breaking change that we don't need right now. The other fix is similar; TypeScript doesn't differentiate between the built-in `WebSocket` type and the `ws` library. Renaming the import solves this too.
jackfranklin
force-pushed
the
upgrade-typescript-3.6
branch
from
29d5aa6
to
b44abf8
Compare
Continues the work to get up to TS 3.8 (latest release at time of
writing).
This version of TS introduced built in definitions for web workers that
include an
interface Workerso TS gets confused when it sees usreference a
Worker. I have renamed the imports toPuppeteerWorkerasI couldn't figure out a way to tell TS to not load in the worker types;
longer term we might consider renaming
WorkertoPuppeteerWorker(oran alternative) but that would be a breaking change that we don't need
right now.
The other fix is to solve an issue where the
wslibrary doesn'timplement a
dispatchEventmethod - which is not a problem - but TSexpects it to. I've used
@ts-ignoreon that error because it'sabsolutely fine and not something we rely on.