• 
      

    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:
    Completed
    Change Summary:
    Pushed to release-6.x (4a62a95)