helpers.each support modules

[souljorje]

Issue

Impossible to pass a javascript module to Chart.register

Description

Rollup build with output.generatedCode.symbols: true or Vite build by default in some cases causes "not registered" errors.

This code:

// registerables.js
export {
    LineElement,
    PointElement,

    LineController,

    CategoryScale,
    LinearScale,

    Tooltip,
} from 'chart.js';

// index.js
import { Chart } from 'chart.js';
import * as registerables from './registerables';
Chart.register({ ...registerables });
export { Chart };

Becomes:

// registerables.js
var rc = Object.freeze({
    __proto__: null,
    [Symbol.toStringTag]: "Module",
    LineElement: se,
    PointElement: Fe,
    LineController: Re,
    CategoryScale: Ae,
    LinearScale: Li,
    Tooltip: Wl
});

Qt.register({ ...rc }) // spreading doesn't help in this case

Error: "category" is not a registered scale. is thrown when I create a line chart.

Because registry uses each which doesn't allow to pass modules, only object https://github.com/chartjs/Chart.js/blob/2031cdf0512a94d2897a682ca156d150f0a83bb3/src/core/core.registry.js#L124-L146 https://github.com/chartjs/Chart.js/blob/2031cdf0512a94d2897a682ca156d150f0a83bb3/src/helpers/helpers.core.ts#L152-L158

Workaround

Till PR isn't merged for those who faced this issue too

Chart.register(
    Object.getOwnPropertyNames(registerables).reduce((acc, key) => {
        acc[key] = rc[key];
        return acc
    }, {});
);

[souljorje]

@kurkle looks like fixing in register requires transforming of the module to the object, feels doubtfully for me.

What if we just add modules support conditionally, so it won't touch current functionality:

export function each<T, TA>(
  loopable: T[] | Record<string, T>,
  fn: (this: TA, v: T, i: any) => void,
  thisArg?: TA,
  reverse?: boolean,
  supportModules?: boolean
) {
  /** ...  */
  } else if (isObject(loopable) || (supportModules && isModule(loopable))) {

[kurkle]

@etimberg @LeeLenaleee @dangreen any thoughts on this?

[LeeLenaleee]

The problem with the supportModules option I forsee is that it is somthing people wont read in the docs, will still put questions in for since people already read the docs badly so fixing this in a way that does not need any configuration is in my oppinion the best.

But as you pointed out it is used in a lot of places so it might have unintended side effects and if we decide to add it, it will be hard to remove it if we dont want it since it gives unintended side effects now or down the road.

[etimberg]

I'm not sure about adding to this helper. In general we've been trying to remove it since the function call overhead can add up a lot during rendering.

[souljorje]

@etimberg you mean that performing supportModules && isModule(loopable) will affect performance? Ok, maybe we could make something like eachWithModules and use it in register? Or I could just inline it in register if you don't feel that it can be reused somewhere else. Kinda not DRY, but won't touch anything else.

[etimberg]

I mean that in general the each helper has caused performance issues so we're not using it for new code. I would just inline the change in register

[dangreen]

My suggestions:

1. Do not export registerables like this. Just export all as object:

import {
    LineElement,
    PointElement,

    LineController,

    CategoryScale,
    LinearScale,

    Tooltip,
} from 'chart.js';

export default {
    LineElement,
    PointElement,

    LineController,

    CategoryScale,
    LinearScale,

    Tooltip,
};

2. Fix isObject helper like this:

export function isObject(value: unknown): value is AnyObject {
 if (value === null) {
    return false;
  }

  const proto = Object.getPrototypeOf(value);
  return proto === null || proto === Object.prototype;
}