-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
fix: make more files work in strict-mode TypeScript #7936
Conversation
Interesting, just merged this in the minor
This is how we are launching Puppeteer:
Should it break this way @OrKoN @mathiasbynens ? |
@karlhorky nope, that's not expected. We try to update Puppeteer to use strict-mode TypeScript and it looks like the types are lying to us here. Let me revert that part. |
actually, I think the types might be correct. I am not able to reproduce locally though but I suspect it is an issue similar to #7932? @karlhorky, could you share a reproduction? |
My colleague @Josehower is working on this now, and will share a reproduction. The basic idea behind the repro is: running a bundled Puppeteer ( |
HI, @OrKoN I am a colleague of @karlhorky thanks for the quick response and the attention to this issue. puppeteer is quite a nice tool that we use every day on our coding Bootcamp. I have created this basic repo with a script that contains puppeteer and a GitHub workflow setup on issue comment. you can find the error documented here.
if you want to test a bit the reproduction steps are:
you can even check the code for the script containing puppeteer v13.1.3 As you can see on the image the error is being ended from line 26364. As additional information, I tried patching puppeteer with the possible solution presented on the PR 7933 whit no success. we use a puppeteer bundled with esbuild and the base file with the script is a ts file. again thanks for your time and looking forward to your answer. Good Energy 👍👍👍👍 |
@Josehower Thanks for the details. Could you try out the following CL #7967 ? |
@OrKoN Thanks ill try it and let you know |
Issues: #6769