fix(Polyline): safeguard points arg from options

[ShaMan123]

Description

Passing the following will result in points2 being set as the polyline points

new Polyline(points1, { points: points2 })

This PR fixes that and adds a test for it

closes #8296

In Action

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

• Run Next Template
• Run Node Template
• Run Vanilla Template
• Run Vue Template

[github-actions]

Build Stats

file / KB (diff)	bundled	minified
fabric	916.896 (+0.023)	305.668 (+0.013)

[github-actions]

Coverage after merging fix/polyline-init into master will be

83.89%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
index.node.ts	14.29%	100%	0%	25%	23, 26, 29, 41, 44, 47
src
ClassRegistry.ts	90.32%	61.54%	100%	97.73%	31, 52
Collection.ts	78.47%	42.62%	87.10%	85.82%	130, 138, 153, 155–157, 159, 169–170, 181, 197, 215, 217, 228, 243, 254, 265, 270, 279, 281, 286–287, 302, 304, 309–310, 329, 333–334, 338–344, 346–348, 350
CommonMethods.ts	89.86%	69.23%	100%	94%	50, 52, 61
Intersection.ts	85.25%	48.91%	100%	97.30%	184–188, 190, 228, 237, 239, 289, 297, 297
Observable.ts	79.89%	54.55%	93.75%	87.10%	136, 145, 148, 160, 162, 167, 68–70, 72, 76, 80, 84–85, 87–91
Point.ts	90.27%	61.22%	100%	93.60%	104, 117, 148, 157, 179, 197, 206, 216, 225, 236–239, 259, 285, 297, 317, 328, 341, 349, 359, 95
Shadow.ts	87.85%	78.26%	100%	88.37%	147, 150, 152–157, 166, 203, 206, 213, 230–237, 241–242, 38–41
cache.ts	84.88%	45.45%	100%	90.14%	57, 59, 71–72, 74–77
config.ts	87.73%	55%	66.67%	94.03%	132, 134–137, 139, 142–143, 147, 152
constants.ts	100%	100%	100%	100%
src/LayoutManager
ActiveSelectionLayoutManager.ts	93.33%	76.92%	85.71%	100%
LayoutManager.ts	90.54%	65.06%	76.92%	99.29%	269, 333, 344
constants.ts	100%	100%	100%	100%
index.ts	48.57%	37.50%	80%	66.67%	1, 1, 1–2, 2, 2–3, 3, 3–4, 4, 4–5, 5, 5, 5–6
types.ts	100%	100%	100%	100%
src/LayoutManager/LayoutStrategies
ClipPathLayout.ts	73.08%	50%	100%	78.95%	39, 41–44, 46–48, 57–58, 66–69
FitContentLayout.ts	100%	100%	100%	100%
FixedLayout.ts	85.71%	20%	100%	100%	23, 23
LayoutStrategy.ts	89.26%	59.26%	100%	97.70%	54, 72, 74
utils.ts	72.58%	50%	100%	78.72%	29–32, 34–35, 40–44
src/Pattern
Pattern.ts	70.18%	90.91%	80%	65.95%	105–107, 114, 118–119, 119–122, 130–138, 140–141, 143, 153–164, 174, 176–181, 183–188, 190–199, 204–205, 207–209, 211, 33, 37
src/brushes
BaseBrush.ts	89.33%	91.67%	100%	88.55%	110, 115, 124–125, 130, 135, 143, 146, 155–160, 99
CircleBrush.ts	52.10%	12.50%	12.50%	58.25%	100–108, 108–118, 122, 130–139, 55, 67, 69, 76, 76, 78–79, 79, 83, 85–86, 92–98
PatternBrush.ts	97.06%	87.50%	100%	100%	21
PencilBrush.ts	91.06%	82.35%	100%	93.81%	122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
SprayBrush.ts	0%	0%	0%	0%	107, 107, 107, 107, 107–108, 110–111, 118–119, 121, 123–127, 136, 140–141, 141, 148, 148, 148–151, 153–156, 160–161, 163, 165–168, 17, 171, 178–179, 18, 181, 183–184, 186, 193–194, 196–197, 20, 200, 200, 207, 207, 21, 211, 22, 22, 22–24, 28, 32, 39, 46, 53, 60, 67, 84–86, 94–96, 98–99
src/canvas
Canvas.ts	75.49%	39.76%	92.86%	84.92%	1004, 1007–1008, 1012–1013, 1018, 1063–1068, 1105, 1126, 1128, 1130, 1137, 1140, 1203–1207, 1286–1290, 1320, 1337, 1383–1400, 1406–1411, 1414–1415, 1417, 1421, 1423–1424, 1426–1428, 1432, 1434, 1436–1438, 1441–1446, 1449–1451, 1454, 1456, 1470, 1477, 1479–1490, 1492–1495, 1495, 1497, 1501–1502, 1505–1506, 1509–1511, 1514, 1522, 354, 369, 388, 443, 559–563, 566–567, 569, 579, 582–583, 585, 588–590, 602, 609–613, 615–620, 622–626, 659, 661, 668–672, 674–679, 681, 683–684, 686–689, 691–692, 747, 783–786, 789, 791, 794, 796, 798, 824, 886–887, 932–933, 935–936, 938, 947–953, 956, 963–964, 966–970, 972, 974, 995–996, 999
CanvasOptions.ts	100%	100%	100%	100%
SelectableCanvas.ts	85.35%	53.54%	100%	94.34%	1012, 1021, 1089–1093, 1139–1140, 1142–1143, 1145, 1172, 1174–1175, 1175, 1177, 1216, 1218, 1220, 1236, 1238, 1242, 1245, 1266, 1269, 1288, 1290, 1294, 1312–1319, 1322, 1325, 1334–1335, 1339–1348, 1352, 1354–1355, 1361–1366, 1370, 362, 384, 462, 513, 515, 591, 596, 673, 701, 992, 994–995
StaticCanvas.ts	75.93%	70.11%	98.91%	75.43%	1003, 1009, 1012, 1014, 1016, 1024, 1026–1032, 1042, 1045, 1048–1054, 1056–1057, 1059–1080, 1087–1089, 1091, 1099, 1105, 1107, 1107–1108, 1108–1109, 1111, 1111–1116, 1129, 1134–1136, 1140, 1144, 1146, 1148, 1153–1157, 1159, 1161–1164, 1167, 1169, 1178, 1180–1181, 1188–1191, 1193, 1199–1202, 1206–1207, 1217, 1223–1225, 1227–1232, 1232, 1232–1236, 1236, 1236–1241, 1243–1250, 1279–1280, 1282–1283, 1285–1287, 1289–1295, 1299, 1301–1314, 1322–1323, 1333, 1344, 1385–1389, 1391, 1393, 1395–1399, 1418, 1433, 1448, 1468, 1501, 1505, 1516–1

[asturur]

Isn't 'fromObject' already doing it safely? how do you get to the bug?

[asturur]

this is fun:

      super(_objectSpread2(_objectSpread2({}, options), {}, {
        points
      }));

vs

      super(_objectSpread2({
        points
      }, options));

[asturur]

I would think this was safeguarding us for all classes

  /**
   *
   * @param {Function} klass
   * @param {object} object
   * @param {object} [options]
   * @param {string} [options.extraParam] property to pass as first argument to the constructor
   * @param {AbortSignal} [options.signal] handle aborting, see https://developer.mozilla.org/en-US/docs/Web/API/AbortController/signal
   * @returns {Promise<FabricObject>}
   */
  static _fromObject<S extends FabricObject>(
    { type, ...object }: Record<string, unknown>,
    { extraParam, ...options }: Abortable & { extraParam?: string } = {}
  ): Promise<S> {
    return enlivenObjectEnlivables<any>(cloneDeep(object), options).then(
      (enlivedMap) => {
        const allOptions = { ...options, ...enlivedMap };
        // from the resulting enlived options, extract options.extraParam to arg0
        // to avoid accidental overrides later
        if (extraParam) {
          const { [extraParam]: arg0, ...rest } = allOptions;
          // @ts-expect-error different signature
          return new this(arg0, rest);
        } else {
          return new this(allOptions);
        }
      }
    ) as Promise<S>;
  }

Of course i don't care to safeguard it twice.

This could also be safeguarded with typings, saying that the props have never 'points' and should be done also for the other classes probably like text, path and so on

[ShaMan123]

It is safeguard by fromObject which was the cause of the original issue we want to resolve. But still if the dev calls new Polyline it is not safeguarded and is expected to be I think.

If you want to avoid object spreads from now on just say so and I will try to avoid

[asturur]

No i don't want to avoid spreads in general.
Spread are loops basically and do not belong in hot code imho, but apart from that this particular case is just a swap in position seem logic to do so, but i would protect those with typings more than code