Ensure AMD headers of dependencies aren't kept in bundle

[iisakkirotko]

Hi!

Recently an issue with using Plotly to render visualizations in our framework, solara was brought to our attention. Because we use some architecture from old jupyter notebook versions together with require.js, plotly.js fails to load. This turns out to be because, despite support for AMD headers in the plotly.js bundle plotly.js itself being removed in #7229, checks of the form

if (typeof define == "function" && define.amd) {
    define(function $AMD$() {
        return context[name2];
    });
}

are still added to included in the plotly.js bundle by esbuild from plotly's dependencies (as can be seen in the files in dist/). For example, in dist/plotly.js, these checks are present through 24 different dependencies.

I think the ultimate cause of the headers still being present is esbuild not properly handling commonJS modules (see evanw/esbuild#1348). The solution here comes from manzt/anywidget#369 (comment). For a better explanation take a look at the comment below (#7367 (comment)).
A potentially better solution might be to migrate from esbuild to rollup, which does handle commonJS modules properly, however that could prove to be more laborious. If that is your preferred solution, let me know - I'm more than happy to lend a hand!

cc: @maartenbreddels, @manzt

Keeping these here as a reminder to myself:

After opening a pull request, developer:

• should create a new small markdown log file using the PR number e.g. 1010_fix.md or 1010_add.md inside draftlogs folder as described in this README, commit it and push.
• should not force push (i.e. git push -f) to remote branches associated with opened pull requests. Force pushes make it hard for maintainers to keep track of updates. Therefore, if required, please fetch upstream/master and "merge" with master instead of "rebase".

[manzt]

I don’t think this is an issue with how esbuild/Rolllup handle CommonJS (i.e., require syntax) but rather a problem with a plotly.js dependency that uses a Universal Module Definition (UMD) wrapper, which attempts to accommodate multiple module formats. esbuild isn’t adding the AMD header; rather, a plotly.js dependency includes one, and Rollup applies more aggressive tree shaking (i.e., dead code elimination) by treating unknown globals (e.g., a global define function introduced by an AMD system) as undefined. While esbuild also performs dead code elimination, it doesn’t make the same assumption. Using esbuild's https://esbuild.github.io/api/#define, you can explicitly declare constant values and control esbuild's treeshaking behavior more precisely.

In my experience, switching to an entirely new bundler for a specific issue like this isn’t worth it. Bundlers are complex, and overhauling one for minor "better" defaults from another can lead to more problems than it solves. There are always trade offs. For instance, Rollup can require multiple third-party plugins to match esbuild’s baseline functionality.

[iisakkirotko]

Thanks a lot for correcting me on the explanation @manzt!

[manzt]

Thanks for tracking down the issue!

[marthacryan]

@iisakkirotko Thank you for the PR! This actually fixes plotly/plotly.py#5027! I just tested locally

[marthacryan]

Tried this out locally and it's working great! Thanks again for this PR @iisakkirotko :D

[archmoj]

Thanks @iisakkirotko for the PR.