thi-ng / umbrella

⛱ Broadly scoped ecosystem & mono-repository of 199 TypeScript projects (and ~180 examples) for general purpose, functional, data driven development
https://thi.ng
Apache License 2.0
3.36k stars 149 forks source link

[geom] rect helper argument parsing fails when attrs is undefined #336

Closed acarabott closed 2 years ago

acarabott commented 2 years ago

What

When undefined is passed to rect for the attrs param, we end up with the wrong pos and size

import { rect, Rect } from "@thi.ng/geom";

const pos = [100, 200];
const size = [300, 400];
const attrs = undefined;

const rect1 = rect(pos, size, attrs);     // pos:     [0, 0], size: [100, 200]  :(
const rect2 = new Rect(pos, size, attrs); // pos: [100, 200], size: [300, 400]  :)
const rect3 = rect(pos, size);            // pos: [100, 200], size: [300, 400]  :)
const rect4 = new Rect(pos, size);        // pos: [100, 200], size: [300, 400]  :)

https://codesandbox.io/s/thing-umbrella-rect-rekked-wjt1l?file=/src/index.ts

Issue

I believe the issue is in geom/internal/args, where args.length === 2 should be args.length === 3.

export const __argsVV = (args) => {
    const attr = __argAttribs(args);
    return args.length
        ? args.length === 2
            ? [args[0], args[1], attr]
            : [undefined, args[0], attr]
        : [undefined, undefined, attr];
};

This function appears to be used by rect, aabb, and ellipse which all have a three argument constructor, so I believe this fix should be appropriate, but want to check before submitting a fix.

postspectacular commented 2 years ago

Hi Arthur, thanks v. much for this & apols for the delay - instead of changing the __argsVV helper, I updated the __argsAttribs() function to also pop a given nullish attrib arg from the array. Also added some tests...

(New version just released :)

acarabott commented 2 years ago

Hey Karsten, looks good. No problem about the delay. Happy to help, as ever!