Skip to content
This repository was archived by the owner on Apr 6, 2021. It is now read-only.

Conversation

@RobinMalfait
Copy link
Member

@RobinMalfait RobinMalfait commented Mar 2, 2021

  1. Re-use one of the selectors in the @apply so that we skip twice the work in replacing that selector.
  2. Fix potential sorting order bug (overflows & undefined behaviour, yay!)

The sort bug could happen once we have ~50 variants, which is not that unrealistic if you think about user defined variants.
I'm saying ~50 because we also have an offset.

       2n ** 55n  // 36028797018963968n
Number(2n ** 55n) // 36028797018963970
//                                  ^ - 68 vs 70

When sorting based on the BigInt values, we are doing `Number(a - z)`,
but if the result of `a - z` is bigger then the max number, then
overflows would happen. This will result in a hard-to-debug bug.

This is a prevention for the future, because currently all our `a - z`
values will not reach Number.MAX_SAFE_INTEGER mark (9007199254740991).
@adamwathan adamwathan merged commit 4ca1c2f into master Mar 2, 2021
@adamwathan adamwathan deleted the bug-fixes branch March 2, 2021 11:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants