[WIP] Try minimizing bundle size

[GeorgianaElena]

This does a couple of things in order to optimize size for production:

• uses an env var NODE_ENV that when set to 'production' should"
  • tell other libraries that this is a production build and use their "optimized" version (at least that's what I understood from the docs)
  • disables source-maps (though webpack recommends these in production also, I've seen many suggestions also to remove these in prod for the sake of size)
• uses CssMinimizerPlugin to minify the css file we import from xterm
• uses the ... syntax to extend existing minimizers (i.e. terser-webpack-plugin), according to the docs

However

The size is only down to 401 KiB :(

Fixes #199 (once it's done)

[consideRatio]

I'm generally confused about this list because previously, jquery wasn't part of it and listed as a devDependency, and now and now jquery is and everything else as well.

My understanding, is that whatever we list under devDependencies, will not be packaged into a bundle - those are just dependencies to help run tests and do the actual packaging into a bundle etc. So, like this, we probably end up packaging webpack into a bundle even though it has no use at all for nbgitpullers JS bundle shipped.

I'm confused in general though. xterm may be needed as a dependency but then why did this work before if they were listed as devDependencies?

[GeorgianaElena]

I don't have an answer sadly. I'm confused about this also... Will come back with ideas once I'll be able to write a coherent sentence about webpack, dependencies and bundles :|

Until then, I'm turning this PR into a draft, as I realized it makes the terminal look awkward again.

[choldgraf]

I wonder if anybody else in jupyterland could help us out here, since there's a lot of JS/webpack experience in the other sub-communities. @bollwyvl helped out with webpack stuff on the pydata theme, maybe he has a suggestion for us here?

[bollwyvl]

Coming in kinda cold but: a lot of this depends on where you're going to be deploying.

If you're in a classic notebook-compatible environment, you'll have all of these things available in the bower components, and would want to use the requirejs machinery to fetch everything you can. These will either already be in cache, or would pre-warm the cache for the successive page load.

In lab, you'd have most of these things available from lab, and it should deduplicate them, but I'm not sure if, e.g. xtermjs is hoisted (should be, as it's pretty big).

Sourcemap size only comes into play at runtime when someone has the developer console open, otherwise they won't go over the wire, and can make it much easier to debug when something does break. However, they do make the underlying wheel/tarball/whatever bigger, so it will be slightly slower at install time.

More broadly: some handy tools for this are the webpack bundle analyzer: https://www.npmjs.com/package/webpack-bundle-analyzer... you can't improve what you can't measure. If you do get to a "target" size, it's probably worth adding a CI check to keep your distribution from going above a certain size... sometimes it's one boolean away!

[consideRatio]

The actual dependencies of the source code part of nbgitpuller is can be structured like this.

  "dependencies": {
    "jquery": "^3.6.0",
    "xterm": "^4.13.0",
    "xterm-addon-fit": "^0.5.0"
  },
  "devDependencies": {
    "css-loader": "^6.2.0",
    "css-minimizer-webpack-plugin": "^3.0.2",
    "mini-css-extract-plugin": "^2.2.0",
    "webpack": "^5.45.1",
    "webpack-cli": "^4.7.2"
  },

Currently in the main branch, but not yet released, jquery, xterm, and xterm-addon-fit are listed under devDependencies. That makes me think that it has been fine to rely on these libraries being available from other locations for anyone having tested nbgitpuller in action so far, because anything in devDependencies doesn't enter the .js file end users installing nbgitpuller get.

  "devDependencies": {
    "jquery": "^3.6.0",
    "webpack": "^5.45.1",
    "webpack-cli": "^4.7.2",
    "xterm": "^4.13.0",
    "xterm-addon-fit": "^0.5.0",
    "css-loader": "^6.2.0",
    "style-loader": "^3.2.1"
  },

I tried doing some research:

• Should nbgitpuller assume the website where it runs have loaded the required javascript libraries already, making it okay to not ship it in its own .js bundle?
• What other .js bundles would have jquery, xterm, xterm-addon-fit that we could rely on indirectly?
  jupyter_server: package.json
  notebook: package.json, bower.json.
  jupyterlab: package.json
  jupyterlab - terminal: package.json
  jupyterlab: jquery doesn't seem to ship with it

@bollwyvl do you have ideas on how / what to package given that nbgitpuller is to support being run on a jupyter_server exposed notebook UI and jupyterlab UI as well as a notebook server exposing a notebook UI or jupyterlab UI?

Note if my question doesn't make sense, it is because I don't understand this well enough to state a question yet 😁

[consideRatio]

The (dev)Dependencies distinction is kinda meaningless in this setting, as they both get smooshed together in node_modules and webpack puts whatever it wants into your resultant bundle unless you tell it otherwise.

Aaaah so due to these lines of code below, we end up having all our required dependencies built into bundle.js created by webpack, while the rest are just node_modules stuff that will be ignored.

nbgitpuller/webpack.config.js

Lines 20 to 25 in 58e61f6

	plugins: [
	new webpack.ProvidePlugin({
	$: 'jquery',
	jQuery: 'jquery',
	}),
	]

nbgitpuller/nbgitpuller/static/js/index.js

Lines 1 to 2 in 58e61f6

	import { Terminal } from 'xterm';
	import { FitAddon } from 'xterm-addon-fit';

[bollwyvl]

Yep. So if you want shiny terminal, you must pay the 200kb. jquery, on the other hand, could be dropped, and contributes just as much. Unless you're supporting IE11, you probably don't need it anymore, and since you're not relying on someone else's jquery, there's nobody else to blame for the 200kb.

But either way: i'd save some pain, and just import $ from 'jquery' rather than relying on magic webpack stuff.

A way to get perceptually improved performance would be to defer loading xterm until it's actually asked for, e.g.

function userActuallyAsksForLogs() {
  const {Terminal} = await import('xterm');
  // do something with the logs
}

If jquery were dropped, and xterm only loaded/drawn if needed, the size of the initial load would be substantially improved.

[yuvipanda]

If this means we can't drop any size at all without rewriting to remove our jquery dependency, then I don't think this is a blocker for 1.0 of nbgitpuller.

[manics]

Is it worth reviewing what xtermjs is used for in this extension, and whether it's needed? One of the main features of xtermjs is supporting interactive input, is this something nbgitpuller requires now (or in the future)?

[bollwyvl]

Kinda seems like a 1.0 is a really good time to address legacy dependencies and performance...

[GeorgianaElena]

Closing this in favor of #272. Thanks everyone on their input ✨