Switch to `esbuild` for faster builds and also to allow using more modern JavaScript syntax and features

[archmoj]

trafficstars

@plotly/plotly_js

TODOs:

• [x] Fix regl_codegen
• [x] Fix custom bundles
• [x] Test new bundle in amdefine
• [x] Test new bundle in requirejs
• [x] Test new bundle in kaleido
• [x] Test new bundle in orca
• [ ] additional QA on the python side on loading the bundle

[birkskyum]

Just want to point out that Rspack is finally in v1.0.0 (link) giving a path forward for speeding webpack projects virtually without code changes. Video here

[archmoj]

@birkskyum, There are now conflicts on this branch after merging your recent PRs. BTW if I recall karma is adjusted with esbuild.

[birkskyum]

I've tried rebasing, but it's quite hard. I think the easiest way to get this PR going again, is to make a new branch off master and apply the needed changes from this PR.

[archmoj]

I've tried rebasing, but it's quite hard. I think the easiest way to get this PR going again, is to make a new branch off master and apply the needed changes from this PR.

Thanks for the note. Let me try fixing it now.

[archmoj]

@birkskyum simply FYI: I noticed an build problem requiring map arcgis styles that were in json format. I fixed it but converting then to js files in 8ba1116.

Then only notable failing test we should care about right now is the requirejs test. That's how plotly.js is imported in plotly.py which is blocking this PR. I thought some of the updates you made e.g. jsdom may help find a fix for this problem. Perhaps there is a problem with UMD header, or arrow functions, or maybe we could make use of requirejs-esm

[birkskyum]

If the requirejs is missing CI tests, what's the step to manually test it?

[archmoj]

If the requirejs is missing CI tests, what's the step to manually test it?

@birkskyum It's not missing on CI. For local testing you could run npm run build && npm run test-requirejs

[birkskyum]

@archmoj , yes it's the amd module wrapper that's missing.

This can be verified by making a separate plotly.min.amd.js file, with this content in the dist folder:

define([], function() {
  const plotly = require('<absolute repo path>/dist/plotly.min');
  return plotly;
});

And loading that instead in test-requirejs:

requirejs.config({
    paths: {
        'plotly': '<absolute repo path>/dist/plotly.min.amd'
    }
});

That'll make the test pass.

Unfortunately esbuild just doesn't have built-in amd output support - see:

• https://github.com/evanw/esbuild/issues/819
• Available options (esm/cjs/iife): https://esbuild.github.io/api/#format

[birkskyum]

Configuring esbuild to make an iife and have the umdwrapper make amd like this:

module.exports = {
    entryPoints: ['./lib/index.js'],
    format: 'iife',
    globalName: 'Plotly',
    bundle: true,
    minify: false,
    sourcemap: false,
    plugins: [
        glsl({
            minify: true,
        }),
        environmentPlugin({
            NODE_DEBUG: false,
        }),
        umdWrapper({
            libraryName: 'Plotly',
            amd: true,
            cjs: true,
            global: true
        })
    ],
    alias: {
        stream: 'stream-browserify',
    },
    define: {
        global: 'window',
    },
    target: 'es2016',
    logLevel: 'info',
};

Gives the outcome that requirejs can load the plotly.min.js bundle like this:

output from test-requirejs

Plotly loaded successfully: {
  version: '3.8.2',
  ascending: [Function: f],
  descending: [Function (anonymous)],
  min: [Function (anonymous)],
  max: [Function (anonymous)],
  extent: [Function (anonymous)],
  sum: [Function (anonymous)],
  mean: [Function (anonymous)],
  quantile: [Function (anonymous)],
  median: [Function (anonymous)],
  variance: [Function (anonymous)],
  deviation: [Function (anonymous)],
  bisectLeft: [Function: left],
  bisectRight: [Function: right],
  bisect: [Function: right],
  bisector: [Function (anonymous)],
  shuffle: [Function (anonymous)],
  permute: [Function (anonymous)],
  pairs: [Function (anonymous)],
  transpose: [Function (anonymous)],
  zip: [Function (anonymous)],
  keys: [Function (anonymous)],
  values: [Function (anonymous)],
  entries: [Function (anonymous)],
  merge: [Function (anonymous)],
  range: [Function (anonymous)],
  map: [Function (anonymous)],
  nest: [Function (anonymous)],
  set: [Function (anonymous)],
  behavior: { drag: [Function (anonymous)], zoom: [Function (anonymous)] },
  rebind: [Function (anonymous)],
  dispatch: [Function (anonymous)],
  event: null,
  requote: [Function (anonymous)],
  selection: [Function (anonymous)] { enter: [Function: Et] },
  ns: {
    prefix: {
      svg: 'http://www.w3.org/2000/svg',
      xhtml: 'http://www.w3.org/1999/xhtml',
      xlink: 'http://www.w3.org/1999/xlink',
      xml: 'http://www.w3.org/XML/1998/namespace',
      xmlns: 'http://www.w3.org/2000/xmlns/'
    },
    qualify: [Function: qualify]
  },
  select: [Function (anonymous)],
  selectAll: [Function (anonymous)],
  mouse: [Function (anonymous)],
  touch: [Function (anonymous)],
  touches: [Function (anonymous)],
  interpolateZoom: [Function (anonymous)],
  color: [Function: Jr],
  hsl: [Function: Ke],
  hcl: [Function: yt],
  lab: [Function: Vr],
  rgb: [Function: mi],
  functor: [Function: ai],
  xhr: [Function (anonymous)],
  dsv: [Function (anonymous)],
  csv: [Function: Fe] {
    parse: [Function (anonymous)],
    parseRows: [Function (anonymous)],
    format: [Function (anonymous)],
    formatRows: [Function (anonymous)]
  },
  tsv: [Function: Fe] {
    parse: [Function (anonymous)],
    parseRows: [Function (anonymous)],
    format: [Function (anonymous)],
    formatRows: [Function (anonymous)]
  },
  timer: [Function (anonymous)] { flush: [Function (anonymous)] },
  round: [Function (anonymous)],
  geom: {
    hull: [Function (anonymous)],
    polygon: [Function (anonymous)],
    voronoi: [Function (anonymous)],
    delaunay: [Function (anonymous)],
    quadtree: [Function (anonymous)]
  },
  interpolateRgb: [Function: al],
  interpolateObject: [Function: Ns],
  interpolateNumber: [Function: rl],
  interpolateString: [Function: Do],
  interpolate: [Function: iu],
  interpolators: [ [Function (anonymous)] ],
  interpolateArray: [Function: fu],
  ease: [Function (anonymous)],
  interpolateHcl: [Function: rv],
  interpolateHsl: [Function: ep],
  interpolateLab: [Function: Bh],
  interpolateRound: [Function: eh],
  transform: [Function (anonymous)],
  interpolateTransform: [Function: av],
  layout: {
    bundle: [Function (anonymous)],
    chord: [Function (anonymous)],
    force: [Function (anonymous)],
    hierarchy: [Function (anonymous)],
    partition: [Function (anonymous)],
    pie: [Function (anonymous)],
    stack: [Function (anonymous)],
    histogram: [Function (anonymous)],
    pack: [Function (anonymous)],
    tree: [Function (anonymous)],
    cluster: [Function (anonymous)],
    treemap: [Function (anonymous)]
  },
  random: {
    normal: [Function: normal],
    logNormal: [Function: logNormal],
    bates: [Function: bates],
    irwinHall: [Function: irwinHall]
  },
  scale: {
    linear: [Function (anonymous)],
    log: [Function (anonymous)],
    pow: [Function (anonymous)],
    sqrt: [Function (anonymous)],
    ordinal: [Function (anonymous)],
    category10: [Function (anonymous)],
    category20: [Function (anonymous)],
    category20b: [Function (anonymous)],
    category20c: [Function (anonymous)],
    quantile: [Function (anonymous)],
    quantize: [Function (anonymous)],
    threshold: [Function (anonymous)],
    identity: [Function (anonymous)]
  },
  svg: {
    arc: [Function (anonymous)],
    line: [Function (anonymous)] { radial: [Function (anonymous)] },
    area: [Function (anonymous)] { radial: [Function (anonymous)] },
    chord: [Function (anonymous)],
    diagonal: [Function (anonymous)] { radial: [Function (anonymous)] },
    symbol: [Function (anonymous)],
    symbolTypes: [
      'circle',
      'cross',
      'diamond',
      'square',
      'triangle-down',
      'triangle-up'
    ],
    axis: [Function (anonymous)],
    brush: [Function (anonymous)]
  },
  transition: [Function (anonymous)],
  text: [Function (anonymous)],
  json: [Function (anonymous)],
  html: [Function (anonymous)],
  xml: [Function (anonymous)]
}

.. but it doesn't have a PlotSchema property on it, so figuring out how to adapt the preprocessing might be the last step.

[archmoj]

@birkskyum That object looks like the d3 object not Plotly. The amd header was present in the build last time I checked it. I suspect this might be related to a PR we accepted in v2 where the this keyword was replaced with self in our d3 fork to make our bundle work with es import.

[birkskyum]

There is in preprocess.js this section:

exposePartsInLib()
function exposePartsInLib() {
     ...

    insert('core', 'src');
 ...
 }

If I undertand that correctly, the src/core.js is inserted, and it has:

...

exports.Snapshot = require('./snapshot');
exports.PlotSchema = require('./plot_api/plot_schema');

[archmoj]

@birkskyum That object looks like the d3 object not Plotly. The amd header was present in the build last time I checked it. I suspect this might be related to a PR we accepted in v2 where the this keyword was replaced with self in our d3 fork to make our bundle work with es import.

One of these PRs: https://github.com/plotly/d3/pulls?q=is%3Apr+is%3Aclosed

Does it work if one loads last d3 v3 from original repo?

[birkskyum]

If I install "d3": "^3.5.17", and swap @plotly/d3 imports, the npm run build break with:

> [email protected] schema
> node tasks/schema.mjs dist

Error: Uncaught [TypeError: Cannot read properties of undefined (reading 'document')]
    at reportException (/Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:66:24)
    at processJavaScript (/Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/living/nodes/HTMLScriptElement-impl.js:240:7)
    at HTMLScriptElementImpl._innerEval (/Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/living/nodes/HTMLScriptElement-impl.js:173:5)
    at /Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/living/nodes/HTMLScriptElement-impl.js:114:12
    at ResourceQueue.push (/Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/browser/resources/resource-queue.js:53:16)
    at HTMLScriptElementImpl._fetchInternalScript (/Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/living/nodes/HTMLScriptElement-impl.js:113:21)
    at HTMLScriptElementImpl._eval (/Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/living/nodes/HTMLScriptElement-impl.js:167:12)
    at HTMLScriptElementImpl._attach (/Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/living/nodes/HTMLScriptElement-impl.js:49:12)
    at HTMLBodyElementImpl._insert (/Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/living/nodes/Node-impl.js:837:14)
    at HTMLBodyElementImpl._preInsert (/Users/admin/repos/plotly.js/node_modules/jsdom/lib/jsdom/living/nodes/Node-impl.js:758:10) TypeError: Cannot read properties of undefined (reading 'document')
    at about:blank:369:32
    at node_modules/d3/d3.js (about:blank:10754:8)
    at __require (about:blank:34:52)
    at src/lib/index.js (about:blank:20007:16)
    at __require (about:blank:34:52)
    at build/plotcss.js (about:blank:20938:17)
    at __require (about:blank:34:52)
    at src/core.js (about:blank:68157:7)
    at __require (about:blank:34:52)
    at lib/core.js (about:blank:68226:24)
file:///Users/admin/repos/plotly.js/tasks/schema.mjs:56
    var obj = Plotly.PlotSchema.get();
                     ^

TypeError: Cannot read properties of undefined (reading 'PlotSchema')
    at makeSchema (file:///Users/admin/repos/plotly.js/tasks/schema.mjs:56:22)
    at file:///Users/admin/repos/plotly.js/tasks/schema.mjs:85:1
    at ModuleJob.run (node:internal/modules/esm/module_job:195:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:337:24)
    at async loadESM (node:internal/process/esm_loader:34:7)
    at async handleMainPromise (node:internal/modules/run_main:106:12)

[archmoj]

How about the intermediate @plotly/d3 versions e.g. 3.8.0?

[birkskyum]

That gives the same as the latest plotly/d3 here

[birkskyum]

The npm run test-jasmine -- surface and parcoords appear to pass everything locally, so I wonder why they fail in ci

[archmoj]

The npm run test-jasmine -- surface and parcoords appear to pass everything locally, so I wonder why they fail in ci

@birkskyum OK. Something strange seems to start happening today. Those jasmine tests pass when I merged your PR. But now if I rerun they fail. This is possibly related to latest Chrome version if we didn't lock it? To possibly confirm that scenario I'm rerunning the tests on master now.

[archmoj]

The npm run test-jasmine -- surface and parcoords appear to pass everything locally, so I wonder why they fail in ci

@birkskyum OK. Something strange seems to start happening today. Those jasmine tests pass when I merged your PR. But now if I rerun they fail. This is possibly related to latest Chrome version if we didn't lock it? To possibly confirm that scenario I'm rerunning the tests on master now.

@birkskyum Yes it failed on master too. So the Chrome version should be pinned on master.

[birkskyum]

This PR takes my local build time from 3m 24.3s down to 12.1s, so going back to the main branch already feels like building in slow motion - well done!

[gvwilson]

17X performance improvement is exciting.

[archmoj]

One could argue it's actually more than that. On CI if we look at CI-build task in isolation it used to take 90 seconds. Now it's only 3 seconds. So 20-30X seems like a better number.

[archmoj]

@alexcjohnson @gvwilson This PR is ready to go :rocket:

[gvwilson]

thank you - @marthacryan can you please have a look at this as well?

[archmoj]

I'm looking forward to @alexcjohnson's review on this as well. Also thanks to @marthacryan and @birkskyum reviews & contributions :pray:

This PR helps unblock Chart2Music keyboard and sound accessibility features in #6680. cc: @Coding-with-Adam

It might be a good idea and timing to include this in the upcoming RC release of [v2.35.0] (https://github.com/plotly/plotly.js/milestone/72) and our QA. cc: @gvwilson @ndrezn @LiamConnors

It would also help our ongoing and future development and testing of other pull requests. cc: @emilykl @antoinerg

[gvwilson]

I approve. Let's hold off on the merge until next week (feels too big to do on a Friday), but congratulations everyone - well done.

[ndrezn]

Super excited about this! This change will really improve the development experience for Plotly.js. Kudos 🥳

[Coding-with-Adam]

Wonderful news 🚀 . Thank you @archmoj; Thank you @marthacryan

[archmoj]

Thanks for the reviews! Before merging this PR, I'd like to do additional QA on the python side on loading the bundle.

[birkskyum]

We'll need to configure esbuild to also allow require(file.css) because of:

• https://github.com/plotly/plotly.js/pull/7140#discussion_r1747626991

One option is here (esbuild-style-plugin), but there might be built-in support for this in esbuild:

• https://github.com/plotly/plotly.js/pull/7142