Support JavaScript modules, modern linting, and rollup.js in Review Board.

Review Request #12777 — Created Jan. 10, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

This updates our JavaScript bundling process in Review Board to use the
new @beanbag/frontend-buildkit Node package, bringing with it
rollup.js as a JavaScript bundler. This allows us to use modern
JavaScript module support, which can help us better organize our code
and move away from setting everything directly in the RB namespace.

Ultimately, anything exported by our bundles will be set in the RB
namespace, but this is now controlled at bundle-time instead of being
assumed in code.

Configuration files for Babel, Rollup, ESLint, and TypeScript have been
set up to ensure we have a working build configuration. These specify
include paths, mappings of import paths to global namespaces (like
djblets/* to Djblets), and lint rules.

A .djblets directory is now set up during setup.py develop to point
to the Djblets package directory, to ensure static media paths to
Djblets are correct. It will be required to run setup.py develop once
after this change, and any time the target Djblets has moved/changed.

Going forward, the plan is to work toward each Pipeline bundle mapping
to a single index.ts file, which would then import and re-export
anything that should be public. This would give rollup.js full control
of all JavaScript bundling, and let Pipeline take that result and make
it compatible with the Django static media system.

In the meantime, we'll continue to have plenty of legacy JavaScript
sitting around modifying RB directly. This is fine, and will not
conflict. We'll need to be thoughtful in migrating this code. We don't
want to just rename and add exports. Instead, for many of these, we
should think about where the modules should truly live first.

A few modules did get updated in this change, due to necessity.
rb/js/common/index.ts was introduced to set some default exports
(Product and EnabledFeatures), so that we could guarantee that RB
was set to something by default during bundle. This replaces the old
window.RB = {} line. Similarly, some nested namespaces have been
exported by other modules (for, e.g., RB.Admin).

Set up some imports/exports between our various bundles, and importing
from Djblets. Verified that the generated code in the browser was all
correct, and that I could package up the resulting JavaScript.

Tested datagrid pages, review request pages, admin UI pages, and the
My Account pages for errors.

Summary ID
Support JavaScript modules, modern linting, and rollup.js in Review Board.
This updates our JavaScript bundling process in Review Board to use the new `@beanbag/frontend-buildkit` Node package, bringing with it rollup.js as a JavaScript bundler. This allows us to use modern JavaScript module support, which can help us better organize our code and move away from setting everything directly in the `RB` namespace. Ultimately, anything exported by our bundles *will* be set in the `RB` namespace, but this is now controlled at bundle-time instead of being assumed in code. Configuration files for Babel, Rollup, ESLint, and TypeScript have been set up to ensure we have a working build configuration. These specify include paths, mappings of import paths to global namespaces (like `djblets/*` to `Djblets`), and lint rules. A `.djblets` directory is now set up during `setup.py develop` to point to the Djblets package directory, to ensure static media paths to Djblets are correct. It will be required to run `setup.py develop` once after this change, and any time the target Djblets has moved/changed. Going forward, the plan is to work toward each Pipeline bundle mapping to a single `index.ts` file, which would then import and re-export anything that should be public. This would give rollup.js full control of all JavaScript bundling, and let Pipeline take that result and make it compatible with the Django static media system. In the meantime, we'll continue to have plenty of legacy JavaScript sitting around modifying `RB` directly. This is fine, and will not conflict. We'll need to be thoughtful in migrating this code. We don't want to just rename and add exports. Instead, for many of these, we should think about where the modules should truly live first. A few modules did get update in this change, due to necessity. `rb/js/common/index.ts` was introduced to set some default exports (`Product` and `EnabledFeatures`), so that we could guarantee that `RB` was set to something by default during bundle. This replaces the old `window.RB = {}` line. Similarly, some nested namespaces have been exported by other modules (for, e.g., `RB.Admin`).
67a19b5cb0f926a88143015bf984c6edc1a20447
Description From Last Updated

Typo in last paragraph of description: "get update" -> "get updated"

daviddavid

Hmm. Rather than specifying this, I feel like we should use the browserslistConfigFile config key.

daviddavid
david
  1. 
      
  2. Show all issues

    Typo in last paragraph of description: "get update" -> "get updated"

  3. .babelrc (Diff revision 1)
     
     
    Show all issues

    Hmm. Rather than specifying this, I feel like we should use the browserslistConfigFile config key.

    1. Yeah, I copied this from elsewhere. We can just omit the settings entirely. It defaults to enabling that.

  4. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (4a62a95)
Loading...