Skip to content

Add an ESM build #414

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jan 5, 2021
+69 −70
Merged

Add an ESM build #414

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3bb7a5a
Typo; coding style consistency
joshkel Dec 30, 2020
685946d
Add ESM build
joshkel Dec 30, 2020
25db650
Additional revisions to ESM build
joshkel Dec 30, 2020
d4fec2c
Update test configuration to match build configuration
joshkel Dec 30, 2020
e709883
Delete 'gulp build' task
joshkel Dec 31, 2020
c559685
Switch to named imports
joshkel Jan 5, 2021
45f5d61
Update/clarify comments
joshkel Jan 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,3 @@ coverage/*

nbproject/*
dist/
chartjs-plugin-zoom.js
Copy link
Collaborator

Choose a reason for hiding this comment

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

you should probably leave these and add the esm files as well for packaging. see my other comment below

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 I'm understanding correctly, we don't need these .gitignore lines or the gulpfile copyDistFilesTask.

The previous configuration invoked Rollup to put the files under dist, then it called copyDistFilesTask to copy the files out of dist into the project root (which is why the files are listed in .gitignore here). But package.json references the copies under dist directory, and the gulpfile's packageTask references the copies under dist (via outDir). I can't tell that the copies in the project root are used at all, so I removed them.

If I've missed something, please let me know.

Copy link
Collaborator

Choose a reason for hiding this comment

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

gulp package looks like it looks for files in the root directory:

chartjs-plugin-zoom/gulpfile.js

Line 64 in 8194ad6

gulp.src([outDir + '*.js', 'LICENSE.md']),

Alternatively, you might be able to update gulp package to look in dist

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 I'm understanding Gulp correctly, it treats outDir as the glob base for outDir + '*.js', so the files from dist are included at the top level of the resulting zip file. Contrast with the explicit base parameter for the samples glob; those are included in a samples subdirectory within the zip file:

chartjs-plugin-zoom/gulpfile.js

Line 68 in 8194ad6

gulp.src('./samples/**/*', {base: '.'})

I tested by running npm run build; npx gulp package in a fresh copy of my branch, and it created a dist/chartjs-plugin-zoom.zip file with contents similar to the releases on GitHub, so I think things are working.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, we might need to update the comments in the packageTask as they threw me off and look like they might be outdated:

// gather "regular" files landing in the package root
// since we moved the dist files one folder up (package root), we need to rewrite
// samples src="../dist/ to src="../ and then copy them in the /samples directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I found that confusing too. I just pushed some comments to try and clarify; feedback welcome.

chartjs-plugin-zoom.min.js
29 changes: 2 additions & 27 deletions gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ var inquirer = require('inquirer');
var semver = require('semver');
var path = require('path');
var fs = require('fs');
var {exec} = require('child_process');
var karma = require('karma');
var merge = require('merge-stream');
var yargs = require('yargs');
Expand All @@ -24,17 +23,6 @@ var srcDir = './src/';
var srcFiles = srcDir + '**.js';
var outDir = './dist/';

function run(bin, args, done) {
var exe = '"' + process.execPath + '"';
var src = require.resolve(bin);
var ps = exec([exe, src].concat(args || []).join(' '));

ps.stdout.pipe(process.stdout);
ps.stderr.pipe(process.stderr);
ps.on('close', () => done());
}

gulp.task('build', gulp.series(rollupTask, copyDistFilesTask));
gulp.task('package', packageTask);
gulp.task('bump', bumpTask);
gulp.task('lint-html', lintHtmlTask);
Expand All @@ -43,20 +31,7 @@ gulp.task('lint', gulp.parallel('lint-html', 'lint-js'));
gulp.task('unittest', unittestTask);
gulp.task('test', gulp.parallel('lint', 'unittest'));
gulp.task('watch', watchTask);
gulp.task('default', gulp.parallel('lint', 'build', 'watch'));

function rollupTask(done) {
run('rollup/dist/bin/rollup', ['-c'], done);
}

/**
* Copy the files from `/dist` to the root directory.
* @todo remove at version 1.0
*/
function copyDistFilesTask() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

removing this will break gulp package so that the release process doesn't work. you might want to leave this but make it run as the first step of package instead of the last step of build

return gulp.src(outDir + '*.js')
.pipe(gulp.dest('./'));
}
gulp.task('default', gulp.parallel('lint', 'watch'));

function packageTask() {
return merge(
Expand Down Expand Up @@ -151,5 +126,5 @@ function unittestTask(done) {
}

function watchTask() {
return gulp.watch(srcFiles, gulp.parallel('lint', 'build'));
return gulp.watch(srcFiles, gulp.parallel('lint'));
}
4 changes: 2 additions & 2 deletions karma.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ module.exports = function(karma) {
// {pattern: 'test/fixtures/**/*.png', included: false},
'node_modules/chart.js/dist/chart.js',
'test/index.js',
'src/plugin.js'
'src/index.js'
].concat(args.inputs),

preprocessors: {
// 'test/fixtures/**/*.js': ['fixtures'],
'test/specs/**/*.js': ['rollup'],
'test/index.js': ['rollup'],
'src/plugin.js': ['sources']
'src/index.js': ['sources']
},

rollupPreprocessor: {
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
"jsdelivr": "dist/chartjs-plugin-zoom.min.js",
"unpkg": "dist/chartjs-plugin-zoom.min.js",
"main": "dist/chartjs-plugin-zoom.js",
"module": "dist/chartjs-plugin-zoom.esm.js",
"repository": {
"type": "git",
"url": "https://github.com/chartjs/chartjs-plugin-zoom.git"
},
"scripts": {
"build": "gulp build",
"build": "rollup -c",
"test": "gulp test"
},
"devDependencies": {
Expand Down
53 changes: 35 additions & 18 deletions rollup.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
const commonjs = require('rollup-plugin-commonjs');
const nodeResolve = require('rollup-plugin-node-resolve');
const terser = require('rollup-plugin-terser').terser;

const pkg = require('./package.json');
const dependencies = Object.keys(pkg.dependencies)
const peerDependencies = Object.keys(pkg.peerDependencies)
const allDependencies = dependencies.concat(peerDependencies)
const dependencies = Object.keys(pkg.dependencies);
const peerDependencies = Object.keys(pkg.peerDependencies);
const allDependencies = dependencies.concat(peerDependencies);

const banner = `/*!
* @license
Expand All @@ -17,19 +18,24 @@ const banner = `/*!
* https://github.com/chartjs/${pkg.name}/blob/master/LICENSE.md
*/`;

const name = 'ChartZoom';
const globals = {
'chart.js': 'Chart',
'chart.js/helpers': 'Chart.helpers',
hammerjs: 'Hammer'
};
allDependencies.push('chart.js/helpers');

module.exports = [
{
input: 'src/plugin.js',
input: 'src/index.js',
output: {
name: 'ChartZoom',
name,
file: `dist/${pkg.name}.js`,
banner: banner,
banner,
format: 'umd',
indent: false,
globals: {
'chart.js': 'Chart',
hammerjs: 'Hammer'
}
globals
},
plugins: [
commonjs({
Expand All @@ -40,17 +46,14 @@ module.exports = [
external: allDependencies
},
{
input: 'src/plugin.js',
input: 'src/index.js',
output: {
name: 'ChartZoom',
name,
file: `dist/${pkg.name}.min.js`,
banner: banner,
banner,
format: 'umd',
indent: false,
globals: {
'chart.js': 'Chart',
hammerjs: 'Hammer'
}
globals
},
plugins: [
commonjs({
Expand All @@ -60,5 +63,19 @@ module.exports = [
terser({output: {comments: 'some'}})
],
external: allDependencies
}
},
{
input: 'src/index.esm.js',
plugins: [
nodeResolve()
],
output: {
name,
file: `dist/${pkg.name}.esm.js`,
banner,
format: 'esm',
indent: false
},
external: allDependencies
},
];
1 change: 1 addition & 0 deletions src/index.esm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export {default as Zoom} from './plugin';
6 changes: 6 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import {Chart} from 'chart.js';
import Zoom from './plugin';

Chart.register(Zoom);

export default Zoom;
33 changes: 15 additions & 18 deletions src/plugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
'use strict';

import Chart from 'chart.js';
import {clone, each, isNullOrUndef, merge} from 'chart.js/helpers';
import Hammer from 'hammerjs';

var helpers = Chart.helpers;

// Take the zoom namespace of Chart
var zoomNS = Chart.Zoom = Chart.Zoom || {};
// Zoom namespace (kept under Chart prior to Chart.js 3)
var zoomNS = {};

// Where we store functions to handle different scale types
var zoomFunctions = zoomNS.zoomFunctions = zoomNS.zoomFunctions || {};
Expand All @@ -21,7 +19,7 @@ function resolveOptions(chart, options) {
deprecatedOptions.zoom = chart.options.zoom;
}
var props = chart.$zoom;
options = props._options = helpers.merge({}, [options, deprecatedOptions]);
options = props._options = merge({}, [options, deprecatedOptions]);

// Install listeners. Do this dynamically based on options so that we can turn zoom on and off
// We also want to make sure listeners aren't always on. E.g. if you're scrolling down a page
Expand All @@ -46,12 +44,12 @@ function resolveOptions(chart, options) {

function storeOriginalOptions(chart) {
var originalOptions = chart.$zoom._originalOptions;
helpers.each(chart.scales, function(scale) {
each(chart.scales, function(scale) {
if (!originalOptions[scale.id]) {
originalOptions[scale.id] = helpers.clone(scale.options);
originalOptions[scale.id] = clone(scale.options);
}
});
helpers.each(originalOptions, function(opt, key) {
each(originalOptions, function(opt, key) {
if (!chart.scales[key]) {
delete originalOptions[key];
}
Expand All @@ -77,7 +75,7 @@ function directionEnabled(mode, dir, chart) {

function rangeMaxLimiter(zoomPanOptions, newMax) {
if (zoomPanOptions.scaleAxes && zoomPanOptions.rangeMax &&
!helpers.isNullOrUndef(zoomPanOptions.rangeMax[zoomPanOptions.scaleAxes])) {
!isNullOrUndef(zoomPanOptions.rangeMax[zoomPanOptions.scaleAxes])) {
var rangeMax = zoomPanOptions.rangeMax[zoomPanOptions.scaleAxes];
if (newMax > rangeMax) {
newMax = rangeMax;
Expand All @@ -88,7 +86,7 @@ function rangeMaxLimiter(zoomPanOptions, newMax) {

function rangeMinLimiter(zoomPanOptions, newMin) {
if (zoomPanOptions.scaleAxes && zoomPanOptions.rangeMin &&
!helpers.isNullOrUndef(zoomPanOptions.rangeMin[zoomPanOptions.scaleAxes])) {
!isNullOrUndef(zoomPanOptions.rangeMin[zoomPanOptions.scaleAxes])) {
var rangeMin = zoomPanOptions.rangeMin[zoomPanOptions.scaleAxes];
if (newMin < rangeMin) {
newMin = rangeMin;
Expand Down Expand Up @@ -183,7 +181,7 @@ function doZoom(chart, percentZoomX, percentZoomY, focalPoint, whichAxes, animat
// Do the zoom here
var zoomMode = typeof zoomOptions.mode === 'function' ? zoomOptions.mode({chart: chart}) : zoomOptions.mode;

// Which axe should be modified when figers were used.
// Which axes should be modified when fingers were used.
var _whichAxes;
if (zoomMode === 'xy' && whichAxes !== undefined) {
// based on fingers positions
Expand All @@ -193,7 +191,7 @@ function doZoom(chart, percentZoomX, percentZoomY, focalPoint, whichAxes, animat
_whichAxes = 'xy';
}

helpers.each(chart.scales, function(scale) {
each(chart.scales, function(scale) {
if (scale.isHorizontal() && directionEnabled(zoomMode, 'x', chart) && directionEnabled(_whichAxes, 'x', chart)) {
zoomOptions.scaleAxes = 'x';
zoomScale(scale, percentZoomX, focalPoint, zoomOptions);
Expand Down Expand Up @@ -254,11 +252,11 @@ function panNumericalScale(scale, delta, panOptions) {
var diff;

if (panOptions.scaleAxes && panOptions.rangeMin &&
!helpers.isNullOrUndef(panOptions.rangeMin[panOptions.scaleAxes])) {
!isNullOrUndef(panOptions.rangeMin[panOptions.scaleAxes])) {
rangeMin = panOptions.rangeMin[panOptions.scaleAxes];
}
if (panOptions.scaleAxes && panOptions.rangeMax &&
!helpers.isNullOrUndef(panOptions.rangeMax[panOptions.scaleAxes])) {
!isNullOrUndef(panOptions.rangeMax[panOptions.scaleAxes])) {
rangeMax = panOptions.rangeMax[panOptions.scaleAxes];
}

Expand Down Expand Up @@ -289,7 +287,7 @@ function doPan(chartInstance, deltaX, deltaY) {
if (panOptions.enabled) {
var panMode = typeof panOptions.mode === 'function' ? panOptions.mode({chart: chartInstance}) : panOptions.mode;

helpers.each(chartInstance.scales, function(scale) {
each(chartInstance.scales, function(scale) {
if (scale.isHorizontal() && directionEnabled(panMode, 'x', chartInstance) && deltaX !== 0) {
panOptions.scaleAxes = 'x';
panScale(scale, deltaX, panOptions);
Expand Down Expand Up @@ -580,7 +578,7 @@ var zoomPlugin = {
chartInstance.resetZoom = function() {
storeOriginalOptions(chartInstance);
var originalOptions = chartInstance.$zoom._originalOptions;
helpers.each(chartInstance.scales, function(scale) {
each(chartInstance.scales, function(scale) {

var scaleOptions = scale.options;
if (originalOptions[scale.id]) {
Expand Down Expand Up @@ -671,5 +669,4 @@ var zoomPlugin = {
}
};

Chart.register(zoomPlugin);
export default zoomPlugin;