Replace Browserify with Rollup #3221
Conversation
| @@ -1,22 +1,22 @@ | |||
| 'use strict'; | |||
|
|
|||
| /* eslint-env node */ | |||
| /* eslint-disable no-var, prefer-arrow-callback */ | |||
There was a problem hiding this comment.
I converted the remaining vars to consts and anonymous functions to arrow functions for consistency.
| entry: './lms/static/scripts/ui-playground/index.js', | ||
| }, | ||
| ]; | ||
| async function buildJS(rollupConfig) { |
There was a problem hiding this comment.
The buildJS, watchJS and buildAndRunTests functions are the same as the gulpfile.js in the client in hypothesis/client#3810, with just changes to the file paths. It might make sense to extract them into a package in future.
| @@ -19,61 +19,65 @@ import { registerIcons } from '@hypothesis/frontend-shared'; | |||
| import iconSet from './icons'; | |||
| registerIcons(iconSet); | |||
|
|
|||
| // Read configuration embedded into page by backend. | |||
| const config = readConfig(); | |||
| function init() { | |||
There was a problem hiding this comment.
The change in this module is just to extract all the code that was at the top level into an init function. In the context of this PR it avoids a warning from Rollup in development builds. In future it will also facilitate writing unit or integration tests of this module.
|
The backend tests are failing due to an issue with pip. That should be fixed with https://github.com/hypothesis/tox-pip-sync/pull/20. |
This avoids a warning from Rollup in development due to Babel's JSX transform generating references to `this` outside of a function/method. It will also enable us to invoke the code with mocks in a test environment in future. The browser extension uses a similar pattern.
Change the LMS frontend to use Rollup for bundling JS. This change was done for the same reasons and using a similar approach to the corresponding change in the Hypothesis client. See hypothesis/client#3810.
|
I found a method to avoid the Node deprecation warning and slightly speed up loading of configs which we might want to apply here too. See hypothesis/client#3840. Will take a look at this next week. |
Convert Rollup configs to native ES modules and load them with `import(...)`. This avoids a deprecation warning from Node triggered by the `loadConfigFile` import. See also hypothesis/client#3840
|
This PR has been updated to match the changes in hypothesis/client#3840, which avoids the Node deprecation warning. |
Change bundling in development and when running tests to use Browserify rather than Rollup. This adapts the changes that were recently made in the lms [1] and client [2] projects. [1] hypothesis/lms#3221 [2] hypothesis/client#3840 This follows the pattern and reuses code from the re
Change bundling in development and when running tests to use Browserify rather than Rollup. This adapts the changes that were recently made in the lms [1] and client [2] projects. [1] hypothesis/lms#3221 [2] hypothesis/client#3840
Change bundling in development and when running tests to use Rollup rather than Browserify. This adapts the changes that were recently made in the lms [1] and client [2] projects. [1] hypothesis/lms#3221 [2] hypothesis/client#3840
Change the LMS frontend to use Rollup for bundling JS. This change was done for the same reasons and using a similar approach to the corresponding change in the Hypothesis client. See hypothesis/client#3810.
As part of the bundling change, I found it necessary to wrap the initialization code for the frontend app in
frontend_apps/index.jsin a function. See the first commit for details.Unlike in client hypothesis/client#3810, the bundles are still generated as classic scripts using an IIFE rather than native ES modules. This is because I think it will be easier to make that change in a separate step in this project.