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

Update website so it works with the latest version of TypeScript #12

Closed
26 opened this issue Jun 12, 2021 · 5 comments
Closed

Update website so it works with the latest version of TypeScript #12

26 opened this issue Jun 12, 2021 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@26
Copy link

26 commented Jun 12, 2021

The current version of screensy-website/screensy.ts does not compile with the latest version of TypeScript (4.3.2). This is related to #5, which was fixed by requiring an older version in the Dockerfile. This fix is not optimal, since it makes it harder to upgrade or to install screensy without using Docker.

@26 26 added the bug Something isn't working label Jun 12, 2021
@Stef-Gijsberts
Copy link
Contributor

The current version of screensy-website/screensy.ts does not compile with the
latest version of TypeScript (4.3.2).

Indeed, compilation with TypeScript 4.3.3 fails with the following error:

screensy.ts:181:52 - error TS2339: Property 'maxFramerate' does not exist on
type 'RTCRtpEncodingParameters'.

181 rtcSendParameters.encodings[0].maxFramerate = 30;

This is most likely caused by the effort to remove types and features which
are not used in any browser. Indeed, at the time of writing,
RTCRtpEncodingParameters.maxFramerate is not used by any browser according to
caniuse.

We have two options:

  1. Remove the line.
  2. Add a // @ts-ignore. This would cause the option to have effect in the future
    if browser developers choose to implement it.

I prefer the first option, removing the line. Just for simplicity, and because we
don't know what effects the property would exactly have were browser developers
to implement it.

@Stef-Gijsberts
Copy link
Contributor

Indeed, at the time of writing, RTCRtpEncodingParameters.maxFramerate is
not used by any browser according to caniuse.

I however just realized that this statement is not proven to be true. For the
Safari browser, the support info is "Unknown". We might have to do a little
testing to see if the property has any effect on Safari.

@Stef-Gijsberts
Copy link
Contributor

Now, two days later, caniuse does report support of
RTCRtpEncodingParameters.maxFramerate on Safari. Therefore, I think we should
keep the line. Thoughts, @26?

We will have to add a // @ts-ignore then, so that TypeScript 4.3 can be used
to compile screensy.

@26
Copy link
Author

26 commented Jun 19, 2021

That sounds good to me.

@Stef-Gijsberts
Copy link
Contributor

Stef-Gijsberts commented Aug 2, 2021

This was merged into development in ccfe9c4 and will be included in the next release.

(link to the specific line)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants