Add JS API tests for calculations #1918
Conversation
| CalculationOperation, | ||
| CalculationOperator, | ||
| CalculationInterpolation, |
There was a problem hiding this comment.
Shall we prefix these with Sass? E.g. SassCalculationOperation
There was a problem hiding this comment.
I don't think it's necessary—they're fairly unlikely to conflict. The reason we prefix the value types is that some of them conflict with built-in JS classes like Number and String. But since these don't, and they aren't Value subtypes so they don't need to follow the same naming convention, I think leaving them unprefixed is fine.
| it("isn't any other type", () => { | ||
| expect(() => calculation.assertBoolean()).toThrow(); | ||
| expect(() => calculation.assertColor()).toThrow(); | ||
| expect(() => calculation.assertFunction()).toThrow(); | ||
| expect(() => calculation.assertMap()).toThrow(); | ||
| expect(calculation.tryMap()).toBe(null); | ||
| expect(() => calculation.assertNumber()).toThrow(); | ||
| expect(() => calculation.assertString()).toThrow(); | ||
| }); |
There was a problem hiding this comment.
Can you add assertCalculation() to the "isn't any other type" tests for all the other types?
There was a problem hiding this comment.
I think I did on 17c55ef, unless I missed something?
There was a problem hiding this comment.
Oh, but they were missing from the dart test suite. I added them in oddbird/dart-sass@6b3b5b8 and oddbird/dart-sass@52ab266
[skip dart-sass]
[skip sass-embedded]
sass/sass#3622
sass/dart-sass#1988
sass/embedded-host-node#237