Remove "cyclic" references #2969
Conversation
Binary Size ReportAffected SDKs
Test Logs
|
| isEqual(other: Operator): boolean { | ||
| return this.name === other.name; | ||
| } | ||
| export const enum Operator { |
There was a problem hiding this comment.
Consider adding a function to convert from string to Operator in lieu of using the as operator at call sites. This function should invoke fail() if conversion fails. This would restore parity with the old code since the following logic from the deleted fromString() method has been lost:
default:
return fail('Unknown FieldFilter operator: ' + op);There was a problem hiding this comment.
Since we are using "const enums" the value of the operator is actually a string in the code. Hence, there is no conversion. We already validate that the input is one of the specific strings before we convert to the Operator type (via validateStringEnum). I changed validateStringEnum to do the casting on behalf of the caller, which means that the validation and the conversion now happens in the same place.
There was a problem hiding this comment.
I will take a look to see if we can enable a lint setting to catch these cyclic references. It would need to specifically target static instance creations. If I find something, I will send that PR your way.
I am, however, planning to add unit tests that would catch these references. See https://github.com/firebase/firebase-js-sdk/pull/2972/files#diff-9d841c6ff52ac4a265de0910aa64844dR62. These tests would fail if we accidentally introduced a cyclic reference. A lint check is still preferable though.
|
Update: I don't think there is a lint check that we can use here. |
Static members of classes introduce something akin to cyclic references, which prevents the Rollup build from detecting that code is unused. A simple example is:
This translates to the following JavaScript code:
In this example, Rollup doesn't know whether
NONE = new Precondition()is side-effect free, and hence keeps the member and the class. This PR removes all static member variables and replaces them either with const enums (which are inlined) or member functions.