[2.8.0] Bundle & non bundle

[madmoizo]

Expected Behavior

Chart.js -> should not import moment
Chart.bundle.js -> should import moment

Current Behavior

Chart.js -> import moment
Chart.bundle.js -> does not import moment

Possible Solution

Remove moment and use native Date :D

Context

from 155ko (v2.7.x) to 413ko (v2.8.x) for my chartjs chunk.
205ko after replacing Chart.js by Chart.bundle.js which is +50ko minified comparing to the v2.7.0
Pluginable approach should be a must-have

Environment

• Chart.js version: 2.8.0

[simonbrunel]

See #6153

[madmoizo]

Well, it answers my question but does not fix the issue described here.

https://www.chartjs.org/docs/latest/getting-started/installation.html?h=bundle

The bundled build includes Moment.js in a single file

in 2.8.0, the Chart.bundle.js does not seem to include moment (but stand-alone do)

That change should be on a major version.

[madmoizo]

tested with:

result:

[simonbrunel]

In 2.8.0, the Chart.bundle.js does not seem to include moment (but stand-alone do)

Chart.bundle.js (534 KB) contains Moment.js built-in while Chart.js (387 KB) doesn't embed Moment.js but does try to require() it. Until 2.8, we used Browserify with a plugin that generated a bug preventing people to use dist/Chart.js with their own version of Moment.js. Since, we migrated to rollup and this bug has been fixed.

For <script> users, it should work as before and for bundler users, require('chart.js') still requires Moment.js, so nothing changed in 2.8.0. However, require('chart.js/dist/Chart.js') may have changed because the (optional) Moment require in dist/Chart.js has been fixed. That means now, you have to explicitly exclude moment from being bundled based on the bundler you are using.

What bundler are you using?

[madmoizo]

I'm using webpack with dynamic import

const Chartjs = await import(/* webpackChunkName: "chartjs" */ '@/node_modules/chart.js/dist/Chart.bundle.js')

On my side, Chart.js with moment = 381ko

Chart.bundle.js = 205ko

[simonbrunel]

I think Chart.bundle.js doesn't contain moment locales, so that's certainly the reason why.

Did you try: https://webpack.js.org/configuration/externals/

// webpack.config.js
module.exports = {
  //...
  externals: {
    moment: 'moment'
  }
};

[madmoizo]

@simonbrunel Yes, I tried with externals and It works, thanks
I can't say I like to change my build config because of a third party library.
Do you plan to make the date library a plugin and use native Date as a default ?

I had this case on another library recently, they fixed a 2,5 years old issue and considered it a bug fix but in the usage it was a breaking change.

[simonbrunel]

I can't say I like to change my build config because of a third party library.

The previous behavior was a bug, and until v3, moment needs to be a dependency (instead of a peer-dependency), so that's currently the proper/official way to do it. I'm glad that it works for you now!

Do you plan to make the date library a plugin ...

2.8.0 introduces adapters to allow users to pick whatever lib they want but until v3, we can't remove Moment.js from the core build. In v3, moment will be an external adapter if still supported.

... and use native Date as a default ?

No, we don't want to implement ourselves the date logic, so the time scale will still rely on a 3rd party lib.

... considered it a bug fix but in the usage it was a breaking change

That was a bug, not a feature, and you relied on it to exclude moment from your build instead of using the webpack externals. I may be wrong but I don't consider that a "breaking" change because it doesn't break your production build but makes it a bit heavier, which I agree is not great but easily fixable.

I don't think anyone realized that the behavior of require('chart.js/dist/Chart.js') changed, else I'm sure we would have find a solution or postponed to v3.

[madmoizo]

That was a bug, not a feature

It's the question here and sure it's not easy to evaluate.
A long running bug (many years) become the normal behavior for a lot of people who started to use the lib and it can be considered as the normal behavior because it's the expected behavior. So, in the usage, not in the library perspective, it can be considered as a breaking change (and even if it does not break anything technically, heavier bundle (size x 3) = slower app load = increased bounce rate, income loss, ...)

It's not a big deal in my case, but it can be for others.

In v3, moment will be an external adapter if still supported.

Glad to hear it, can't wait for v3!

Thanks for your time & your work on this lib!