Bug: typescript declaration

[Mister-Hope]

Why are you using ScreenFull | { isEnabled: false}, it's causing issues.

E.g: below is a direct copy from d.tsfile:

    if (screenfull.isEnabled) {
      screenfull.on('change', () => {
        console.log('Am I fullscreen?', screenfull.isFullscreen ? 'Yes' : 'No');
      });
    }

and will throw an error:

Property 'isFullscreen' does not exist on type 'Screenfull | { isEnabled: false; }'.
  Property 'isFullscreen' does not exist on type '{ isEnabled: false; }'.

This means either I checked screenfull.isEnabled every time I access it

    if (screenfull.isEnabled) {
      screenfull.on('change', () => {
        console.log('Am I fullscreen?', screenfull.isEnabled && screenfull.isFullscreen ? 'Yes' : 'No');
      });
    }

or I cast it every time:

    if (screenfull.isEnabled) {
      screenfull.on('change', () => {
        console.log('Am I fullscreen?', (screenfull as ScreenFull).isFullscreen ? 'Yes' : 'No');
      });
    }

    if (screenfull.isEnabled) {
      screenfull.on('change', () => {
        // will throw an error if I am in a strict env with `noImplitAny: true`.
        console.log('Am I fullscreen?', (screenfull as any).isFullscreen ? 'Yes' : 'No');
      });
    }

Above error will appear in every method of screenfull.

You may write this to force the users to check isEnabled first, but we have to check it again in every method of screenfull. Please consider changing the ScreenFull | { isEnabled: false}, as the type declaration now is really annoying.

[chybisov]

@sindresorhus same issue, any estimation to fix? Thanks!

[feliperobledo]

First of all, thanks for making this package. I've found it very helpful in the past :)
To add to the description above, I've had to force my code to also do:

(screenful as Screenful).toggle(...)
...
(screenful as Screenful).isFullscreen

It might just be that I have wrong settings in my tsconfig, but this at least forces the compiler to find the right type definition so I can carry forward. I shouldn't have to do this, though :/.

[ciriousjoker]

Less invasive variant that doesn't cause lots of changes once this is fixed:

import * as _screenfull from "screenfull";
import { Screenfull } from "screenfull";
const screenfull = _screenfull as Screenfull;

[sindresorhus]

This is unfortunately a limitation of TypeScript. TS has type guards for things like this, but that doesn't work with properties. Ideally, we would have been able to do specify something like this:

readonly isEnabled: this is Screenfull;

And then Screenfull would be available if you checked the .isEnabled property in an if-statement.

A possible solution:

-declare const screenfull: screenfull.Screenfull | {isEnabled: false};
+declare const screenfull: screenfull.Screenfull | (Partial<screenfull.Screenfull> & {isEnabled: false});

(Untested)

This way it would stay safe, but if you know you already checked for access, you can simply do screenfull.request!(…). Notice the !. No need to cast.

[Mister-Hope]

This is unfortunately a limitation of TypeScript. TS has type guards for things like this, but that doesn't work with properties. Ideally, we would have been able to do specify something like this:

readonly isEnabled: this is Screenfull;

And then Screenfull would be available if you checked the .isEnabled property in an if-statement.

A possible solution:

-declare const screenfull: screenfull.Screenfull | {isEnabled: false};
+declare const screenfull: screenfull.Screenfull | (Partial<screenfull.Screenfull> & {isEnabled: false});

(Untested)

This way it would stay safe, but if you know you already checked for access, you can simply do screenfull.request!(…). Notice the !. No need to cast.

The solution is definitely not working, when you access screenFull in a callback function, ts would assume the screenfull may change when you invoke the callback after you check isEnable.

Also it's not a typescript problem at all. As long as isEnable can be false, typescript will think it may changed to false by the time you invoke the callback defined in methods.

[Mister-Hope]

This is a possible workaround:

if (screenfull.isEnabled) {
  const realScreenFull = screenfull;
  realScreenFull.on('change', () => {
    console.log('Am I fullscreen?', realScreenFull .isFullscreen ? 'Yes' : 'No');
  });
}

As @sindresorhus seems to prefer remaining the isEnabled check, this is probably the only good workaround.

A good typescript code disencourage you use the non-null assertion !., this problem is actully due to design(That's why I gave the 👎 ). You should not export a changable interface in typescript. Instead, you should design api like this:

declare const canUse = () => ScreenFull | false;

import { canUse } from 'screen-full';

const screenFull = canUse();

if (screenFull) {
  .... // screenFull is ScreenFull and is unchangable now
}

[sindresorhus]

You should not export a changable interface in typescript.

The TS definition just reflects the actual JS code.

A good typescript code disencourage you use the non-null assertion !.

You should ideally not use it a lot, but it does make sense when you can be sure it's safe. It is especially useful now that we have the --noUncheckedIndexedAccess flag.

[sindresorhus]

I'm ok with removing the {isEnabled: false}. It seems to cause more problems than the safety it adds.

[piotrkabacinski]

I'm ok with removing the {isEnabled: false}. It seems to cause more problems than the safety it adds.

Hi @sindresorhus, maybe I missed something, but was this fix released? 🤔 This declaration's still in the main branch: https://github.com/sindresorhus/screenfull.js/blob/main/src/screenfull.d.ts#L169

[aradalvand]

Hi @sindresorhus.
This is just causing headaches and not helping anybody. Why did you guys have to be so "clever" about the typings?
In addition to the errors, the intellisense seems to fail to work properly as well, at least in VS Code:

It's perfectly clear at this point that #154 turned out to be a bad idea. It was added as a nice and simple little improvement, but it's causing various problems and a lot of confusion. I, for example, finally found this issue after beating my head against the wall for almost an hour, I thought there was something wrong with my code editor, I even suspected that this was a TypeScript bug, etc.

Please revert it back. Thank you.