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

esbuild gives comparison to -0 compiling firestore #3814

Closed
ojanvafai opened this issue Sep 19, 2020 · 2 comments · Fixed by #3820
Closed

esbuild gives comparison to -0 compiling firestore #3814

ojanvafai opened this issue Sep 19, 2020 · 2 comments · Fixed by #3820

Comments

@ojanvafai
Copy link

  • Operating System version: N/A
  • Browser version: N/A
  • Firebase SDK version: 7.16.0
  • Firebase Product: firestore

esbuild gives the following error compiling firestore:

node_modules/@firebase/firestore/dist/index.cjs.js:718:11: warning: Comparison with -0 using the === operator will also match 0
    return -0 === t && 1 / t == -1 / 0;

Technically both 0 and -0 work the same with === operator. The esbuild warning seems reasonable in general. Can we change the -0 to a 0? In some ways that's also more clear since -0 indicates that it needs to be -0 for the code to work.

Looks like this was added in 4f1f303, which points to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is, which does not use -0.

So the proposal is to change the current code:

export function isNegativeZero(value: number) : boolean {
  return value === -0 && 1 / value === 1 / -0;
}

To the following:

export function isNegativeZero(value: number) : boolean {
  return value === 0 && 1 / value === 1 / -0;
}
@schmidt-sebastian
Copy link
Contributor

@ojanvafai Thanks for letting us know about this. This is certainly something we can fix. I will prepare a PR right away.

@ojanvafai
Copy link
Author

Wow. 3 days since filing! I had low hopes this had any chance of getting fixed at all. Thanks. :)

@firebase firebase locked and limited conversation to collaborators Oct 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants