Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Name artifact chart.umd.js, fixes #11455 #11457

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/bug.yml
Expand Up @@ -45,7 +45,7 @@ body:
For typescript issues you can make use of [this TS Playground](https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAbzgYQBYENZwL5wGZQQhwDkAxhrAHQBWAziQNwCwAUGwG6ZxkwAecALxwAJhDIBXEAFMAdjCoBzaTACiAG2kz5AIQCeASREAKAEQg9aTDFMBKOOjpwAEgBUAsgBlk6WVzoaWnIwLKxcUHAWVljCstIA7iiUMMa8fAA0iGxwOXAwemDSAFyk6sBxJOnZuSLoMOglCNW5ueroAEbS6nQlANqmAErSIqaZpjrqEtKjcKYAml3qEPEzpgDiUNJyqwAKElBgmqsA8lC+yqYAulWsLS219XQqPXC9Tbd3n22d6iUkAMRwCB4OAANQgMGkDBun0+DwarwAjAAmTKIgCcmQAzJkAKyZVFwLHXZp3bCXUnYGG5CBgGDACCyF7vT50MjoTTM0ktPiNbl3fk5KmCuB6PkfWFwEXYfkyiU4NjYWyMIA) to make a reproducible sample.

If filing a bug against `master`, you may reference the latest code via
https://www.chartjs.org/dist/master/chart.umd.js (changing the filename to
https://www.chartjs.org/dist/master/chart.umd.min.js (changing the filename to
point at the file you need as appropriate). Do not rely on these files for
production purposes as they may be removed at any time.
validations:
Expand Down
2 changes: 1 addition & 1 deletion docs/developers/contributing.md
Expand Up @@ -74,6 +74,6 @@ Guidelines for reporting bugs:

- Check the issue search to see if it has already been reported
- Isolate the problem to a simple test case
- Please include a demonstration of the bug on a website such as [JS Bin](https://jsbin.com/), [JS Fiddle](https://jsfiddle.net/), or [Codepen](https://codepen.io/pen/). ([Template](https://codepen.io/pen?template=wvezeOq)). If filing a bug against `master`, you may reference the latest code via <https://www.chartjs.org/dist/master/chart.umd.js> (changing the filename to point at the file you need as appropriate). Do not rely on these files for production purposes as they may be removed at any time.
- Please include a demonstration of the bug on a website such as [JS Bin](https://jsbin.com/), [JS Fiddle](https://jsfiddle.net/), or [Codepen](https://codepen.io/pen/). ([Template](https://codepen.io/pen?template=wvezeOq)). If filing a bug against `master`, you may reference the latest code via <https://www.chartjs.org/dist/master/chart.umd.min.js> (changing the filename to point at the file you need as appropriate). Do not rely on these files for production purposes as they may be removed at any time.

Please provide any additional details associated with the bug, if it's browser or screen density specific, or only happens with a certain configuration or data.
6 changes: 3 additions & 3 deletions docs/getting-started/integration.md
Expand Up @@ -7,7 +7,7 @@ If you're using a front-end framework (e.g., React, Angular, or Vue), please see
## Script Tag

```html
<script src="path/to/chartjs/dist/chart.umd.js"></script>
<script src="path/to/chartjs/dist/chart.umd.min.js"></script>
<script>
const myChart = new Chart(ctx, {...});
</script>
Expand Down Expand Up @@ -122,10 +122,10 @@ const { Chart } = await import('chart.js');

## RequireJS

**Important:** RequireJS can load only [AMD modules](https://requirejs.org/docs/whyamd.html), so be sure to require one of the UMD builds instead (i.e. `dist/chart.umd.js`).
**Important:** RequireJS can load only [AMD modules](https://requirejs.org/docs/whyamd.html), so be sure to require one of the UMD builds instead (i.e. `dist/chart.umd.min.js`).

```javascript
require(['path/to/chartjs/dist/chart.umd.js'], function(Chart){
require(['path/to/chartjs/dist/chart.umd.min.js'], function(Chart){
const myChart = new Chart(ctx, {...});
});
```
Expand Down
2 changes: 1 addition & 1 deletion docs/migration/v4-migration.md
Expand Up @@ -30,7 +30,7 @@ A number of changes were made to the configuration options passed to the `Chart`
* Time and timeseries scales use `ticks.stepSize` instead of `time.stepSize`, which has been removed.
* `maxTickslimit` wont be used for the ticks in `autoSkip` if the determined max ticks is less then the `maxTicksLimit`.
* `dist/chart.js` has been removed.
* `dist/chart.min.js` has been renamed to `dist/chart.umd.js`.
* `dist/chart.min.js` has been renamed to `dist/chart.umd.min.js`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should edit the migration guide.

Since if you look at the latest docs but use version 4.3 this won't be correct.

We can add a (`dist/chart.umd.min.js` after version 4.5.0).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they're upgrading to an older version won't they be seeing that older version's migration guide?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if you click the right link.

But even if you go to 4.5.0 this change would still be technically incorrect since we did not rename it from old to that we renamed it to what's already there and this gets added

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are arguments for both approaches. And I support either.

Here is a PR for the other wording: #11470

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be more in favour of that one, but it wont be a dealbreaker for me.

Seems the test in both pr's fail on that it can't load 'src/chart.umd.min.js':
Uncaught NetworkError: Failed to execute 'importScripts' on 'WorkerGlobalScope': The script at 'src/chart.umd.min.js' failed to load. thrown

* `dist/chart.esm.js` has been renamed to `dist/chart.js`.

#### Type changes
Expand Down
5 changes: 3 additions & 2 deletions package.json
Expand Up @@ -8,10 +8,11 @@
"sideEffects": [
"./auto/auto.js",
"./auto/auto.cjs",
"./dist/chart.umd.min.js",
"./dist/chart.umd.js"
],
"jsdelivr": "./dist/chart.umd.js",
"unpkg": "./dist/chart.umd.js",
"jsdelivr": "./dist/chart.umd.min.js",
"unpkg": "./dist/chart.umd.min.js",
"main": "./dist/chart.cjs",
"module": "./dist/chart.js",
"exports": {
Expand Down
16 changes: 15 additions & 1 deletion rollup.config.js
Expand Up @@ -45,7 +45,21 @@ const plugins = (minify) =>

export default [
// UMD build
// dist/chart.umd.js
// dist/chart.umd.min.js
{
input: 'src/index.umd.ts',
plugins: plugins(true),
output: {
name: 'Chart',
file: 'dist/chart.umd.min.js',
format: 'umd',
indent: false,
sourcemap: true,
},
},

// UMD build
// dist/chart.umd.js (old filename)
{
input: 'src/index.umd.ts',
plugins: plugins(true),
Expand Down
2 changes: 1 addition & 1 deletion test/BasicChartWebWorker.js
Expand Up @@ -6,7 +6,7 @@
// Sends messages with data of types: { type: 'success' } | { type: 'error', errorMessage: string }

// eslint-disable-next-line no-undef
importScripts('../src/chart.umd.js');
importScripts('../src/chart.umd.min.js');

onmessage = function(event) {
try {
Expand Down