Wasm: update CrossSection.toPolygons() return value to match type #963
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #963 +/- ##
==========================================
- Coverage 91.84% 86.76% -5.08%
==========================================
Files 37 61 +24
Lines 4976 8492 +3516
Branches 0 1044 +1044
==========================================
+ Hits 4570 7368 +2798
- Misses 406 1124 +718 ☔ View full report in Codecov by Sentry. |
|
|
||
| suite('CrossSection Bindings', () => { | ||
| test('ToPolygons return correct shape', () => { | ||
| const polygons = manifoldModule.CrossSection.square().toPolygons(); |
There was a problem hiding this comment.
So, is the CI running this test yet? Our current JS tests are in the form of examples, but we could certainly use better coverage.
There was a problem hiding this comment.
Looking at the actions, seems like its running since vitest should should be running *.test.* files by default
| @@ -190,7 +190,7 @@ Module.setup = function() { | |||
|
|
|||
| Module.CrossSection.prototype.toPolygons = function() { | |||
| const vec = this._ToPolygons(); | |||
| const result = vec2polygons(vec); | |||
| const result = vec2polygons(vec, v => [v.x, v.y]); | |||
There was a problem hiding this comment.
Just to double-check, which API do we like better? The one we implemented or the one we documented? Because we could equally update the type instead of this.
There was a problem hiding this comment.
Personally, since there is a static factory function called ofPolygons, I would assume that toPolygons would return the same type.
|
Thanks, I keep forgetting to run format |
|
Also this slipped my mind, but I guess this would count as a breaking change for the npm package? although its a ts bug fix, if someone was using this in js, it would break |
|
Yes, good timing, as we're stacking up breaking changes right now anyway for v3.0. |
Closes #943
I used the existing support for a map callback in
vec2polygons.Also started with a simple test for bindings. I guess the hard part of writing the tests is trying to test the custom bindings and not emscripten or the c++ code.