Skip to content

navigator.vibrate is typed as always being defined, but is undefined in Safari #1486

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

Open
FuXiuHeng opened this issue Jan 31, 2023 · 11 comments

Comments

@FuXiuHeng
Copy link

The navigator.vibrate API is currently always defined. However, according to the navigator.vibrate MDN docs, this is currently not supported in Safari:

Screen Shot 2023-01-31 at 1 58 36 pm

I'm proposing to update the Navigator interface to:

interface Navigator {
    // Proposed change from being required to being optional
    vibrate?: (pattern: Iterable<number>) => boolean;
}
@orta
Copy link
Contributor

orta commented Jan 31, 2023

Yeah, I agree, this is probably the right call (having made this exact mistake myself also)

@HolgerJeromin
Copy link
Contributor

https://github.com/microsoft/TypeScript-DOM-lib-generator#why-is-my-fancy-api-still-not-available-here
We have the policy to need two js engine support.
Perhaps an API with only two engines chould be marked as optional...

@saschanaz
Copy link
Contributor

Yeah, I'd like to have some policy rather than doing special treatment to a specific API.

@orta
Copy link
Contributor

orta commented Feb 3, 2023

Hah, yeah, you're both not wrong there - should probably not do it

@saschanaz
Copy link
Contributor

Oh I mean, I'm open to this but I'd like to modify the policy so that things can be consistent for other APIs too.

That said, not sure how the impact would be.

@HolgerJeromin
Copy link
Contributor

Yeah, my suggestion was to make it optional in the emitter code (where we handle the number of implementations anyway).

That said, not sure how the impact would be.

This would probably make quite a few APIs optional with some breakings... Some good, some bad (depending on the target audience of the customer code)

@orta
Copy link
Contributor

orta commented Feb 3, 2023

Yeah - while I don't think this is a massively used API, I'm willing to bet that systemically making a bunch of fns optional because it's only two is probably going to /feel/ like too much of a breaking change

Hard to say without someone showing a .d.ts diff though

@RyanCavanaugh
Copy link
Member

I think our longstanding policy of marking things as fully present once they're in the relevant spec plus two major browsers is a sensible one, Safari's eccentricities notwithstanding. I realize Safari isn't exactly niche, but "a browser that exists doesn't support a spec'd, otherwise-available API" is not a road that goes to a good place.

@guillaumebrunerie
Copy link

I've been bitten by that before as well (with requestIdleCallback, which is also not available in Safari). But I'm not sure making it optional is the right decision either, ideally you would rather use a polyfill and then it's not optional anymore, rather than testing for its existence every time you want to use it. But I wish there were a way to remind us of which polyfills we may need to add.

@HolgerJeromin
Copy link
Contributor

Perhaps https://github.com/amilajack/eslint-plugin-compat could be an alternative.
But I have not tested it.

@guillaumebrunerie
Copy link

eslint-plugin-compat looks interesting, unfortunately it has some pretty serious issues like exempting everything inside an if statement and not supporting some APIs like requestIdleCallback. For instance the following code does not trigger it at all, even though it would crash twice in Safari:

window.requestIdleCallback(() => {});

const shouldVibrate = true;
if (shouldVibrate) {
    navigator.vibrate();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants