From a2323797f529e4af35d7d67a43dddf6cd88a6a8c Mon Sep 17 00:00:00 2001 From: Brian Jacobel Date: Tue, 11 Apr 2017 16:39:39 -0400 Subject: [PATCH] Work on sandbox bundling for webpack --- cms/envs/bok_choy.py | 3 --- cms/envs/common.py | 1 + common/static/common/js/karma.common.conf.js | 2 +- lms/envs/aws.py | 1 + lms/envs/bok_choy.env.json | 1 - lms/envs/bok_choy.py | 3 --- lms/envs/common.py | 4 ++-- lms/envs/test.py | 1 + lms/envs/test_static_optimized.py | 1 + lms/static/lms/js/build.js | 2 +- .../course_experience/js/CourseOutline.js | 6 +++--- .../js/spec/CourseOutline_spec.js | 16 ++++++++-------- .../course-outline-fragment.html | 2 +- package.json | 16 ++++++++-------- pavelib/assets.py | 15 +++++++++------ pavelib/paver_tests/test_servers.py | 18 +++++++++++++++--- pavelib/utils/envs.py | 19 +++++++++++++++++++ scripts/jenkins-common.sh | 3 +++ webpack.config.js | 9 +++++++-- 19 files changed, 81 insertions(+), 42 deletions(-) diff --git a/cms/envs/bok_choy.py b/cms/envs/bok_choy.py index ae848ac284..1f06621162 100644 --- a/cms/envs/bok_choy.py +++ b/cms/envs/bok_choy.py @@ -27,9 +27,6 @@ from .aws import * # pylint: disable=wildcard-import, unused-wildcard-import ######################### Testing overrides #################################### -# Needed for the reset database management command -INSTALLED_APPS += ('django_extensions',) - # Redirect to the test_root folder within the repo TEST_ROOT = REPO_ROOT / "test_root" GITHUB_REPO_ROOT = (TEST_ROOT / "data").abspath() diff --git a/cms/envs/common.py b/cms/envs/common.py index 7424af8803..a3c4597eae 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -861,6 +861,7 @@ INSTALLED_APPS = ( # Maintenance tools 'maintenance', + 'django_extensions', # Tracking 'track', diff --git a/common/static/common/js/karma.common.conf.js b/common/static/common/js/karma.common.conf.js index cdacc8a6c2..0cf9b43532 100644 --- a/common/static/common/js/karma.common.conf.js +++ b/common/static/common/js/karma.common.conf.js @@ -170,7 +170,7 @@ function junitSettings(config) { * @param {String} pattern * @return {String} */ -// I'd like to change fix the no-shadow violation on the next line, but it would break this shared conf's API. +// I'd like to fix the no-shadow violation on the next line, but it would break this shared conf's API. function defaultNormalizeFunc(appRoot, pattern) { // eslint-disable-line no-shadow if (pattern.match(/^common\/js/)) { pattern = path.join(appRoot, '/common/static/' + pattern); diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 526b733eb7..ebfacc1662 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -121,6 +121,7 @@ with open(CONFIG_ROOT / CONFIG_PREFIX + "env.json") as env_file: STATIC_ROOT_BASE = ENV_TOKENS.get('STATIC_ROOT_BASE', None) if STATIC_ROOT_BASE: STATIC_ROOT = path(STATIC_ROOT_BASE) + WEBPACK_LOADER['DEFAULT']['STATS_FILE'] = STATIC_ROOT / "webpack-stats.json" # STATIC_URL_BASE specifies the base url to use for static files diff --git a/lms/envs/bok_choy.env.json b/lms/envs/bok_choy.env.json index 493e2add6c..18dd05541d 100644 --- a/lms/envs/bok_choy.env.json +++ b/lms/envs/bok_choy.env.json @@ -134,7 +134,6 @@ "SERVER_EMAIL": "devops@example.com", "SESSION_COOKIE_DOMAIN": null, "SITE_NAME": "localhost:8003", - "STATIC_ROOT_BASE": "** OVERRIDDEN **", "STATIC_URL_BASE": "/static/", "SUPPORT_SITE_LINK": "https://support.example.com", "SYSLOG_SERVER": "", diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index 6542ed85f5..48fd2ec05a 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -31,9 +31,6 @@ from .aws import * # pylint: disable=wildcard-import, unused-wildcard-import ######################### Testing overrides #################################### -# Needed for the reset database management command -INSTALLED_APPS += ('django_extensions',) - # Redirect to the test_root folder within the repo GITHUB_REPO_ROOT = (TEST_ROOT / "data").abspath() LOG_DIR = (TEST_ROOT / "log").abspath() diff --git a/lms/envs/common.py b/lms/envs/common.py index d21f589e94..240847f147 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1757,11 +1757,10 @@ REQUIRE_JS_PATH_OVERRIDES = { WEBPACK_LOADER = { 'DEFAULT': { 'BUNDLE_DIR_NAME': 'bundles/', - 'STATS_FILE': os.path.join(REPO_ROOT, 'webpack-stats.json'), + 'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json') } } - ########################## DJANGO DEBUG TOOLBAR ############################### # We don't enable Django Debug Toolbar universally, but whenever we do, we want @@ -2037,6 +2036,7 @@ INSTALLED_APPS = ( 'django.contrib.admin', # only used in DEBUG mode 'django_nose', 'debug', + 'django_extensions', # Discussion forums 'django_comment_client', diff --git a/lms/envs/test.py b/lms/envs/test.py index 44e4431f42..14bb689872 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -111,6 +111,7 @@ NOSE_PLUGINS = [ TEST_ROOT = path("test_root") # Want static files in the same dir for running on jenkins. STATIC_ROOT = TEST_ROOT / "staticfiles" +WEBPACK_LOADER['DEFAULT']['STATS_FILE'] = STATIC_ROOT / "webpack-stats.json" STATUS_MESSAGE_PATH = TEST_ROOT / "status_message.json" diff --git a/lms/envs/test_static_optimized.py b/lms/envs/test_static_optimized.py index 911a05274e..618e39a438 100644 --- a/lms/envs/test_static_optimized.py +++ b/lms/envs/test_static_optimized.py @@ -53,6 +53,7 @@ LOG_DIR = (TEST_ROOT / "log").abspath() # Store the static files under test root so that they don't overwrite existing static assets STATIC_ROOT = (TEST_ROOT / "staticfiles" / "lms").abspath() +WEBPACK_LOADER['DEFAULT']['STATS_FILE'] = STATIC_ROOT / "webpack-stats.json" # Disable uglify when tests are running (used by build.js). # 1. Uglify is by far the slowest part of the build process diff --git a/lms/static/lms/js/build.js b/lms/static/lms/js/build.js index c9a7098d1e..536d867c4d 100644 --- a/lms/static/lms/js/build.js +++ b/lms/static/lms/js/build.js @@ -18,7 +18,7 @@ * done. */ modules: getModulesList([ - 'course_bookmarks/js/course_bookmarks_factory',, + 'course_bookmarks/js/course_bookmarks_factory', 'discussion/js/discussion_board_factory', 'discussion/js/discussion_profile_page_factory', 'js/api_admin/catalog_preview_factory', diff --git a/openedx/features/course_experience/static/course_experience/js/CourseOutline.js b/openedx/features/course_experience/static/course_experience/js/CourseOutline.js index 7c664788f2..246b43afa3 100644 --- a/openedx/features/course_experience/static/course_experience/js/CourseOutline.js +++ b/openedx/features/course_experience/static/course_experience/js/CourseOutline.js @@ -1,6 +1,6 @@ /* globals Logger */ -// import constants from 'edx-ui-toolkit/src/js/utils/constants'; +import { keys } from 'edx-ui-toolkit/src/js/utils/constants'; // @TODO: Figure out how to make webpack handle default exports when libraryTarget: 'window' export class CourseOutline { // eslint-disable-line import/prefer-default-export @@ -11,11 +11,11 @@ export class CourseOutline { // eslint-disable-line import/prefer-default-expor const index = focusable.indexOf(event.target); switch (event.key) { // eslint-disable-line default-case - case 'ArrowDown': // @TODO: Get these from the UI Toolkit + case keys.down: event.preventDefault(); focusable[Math.min(index + 1, focusable.length - 1)].focus(); break; - case 'ArrowUp': // @TODO: Get these from the UI Toolkit + case keys.up: // @TODO: Get these from the UI Toolkit event.preventDefault(); focusable[Math.max(index - 1, 0)].focus(); break; diff --git a/openedx/features/course_experience/static/course_experience/js/spec/CourseOutline_spec.js b/openedx/features/course_experience/static/course_experience/js/spec/CourseOutline_spec.js index 862b6eb169..faa60eb9ad 100644 --- a/openedx/features/course_experience/static/course_experience/js/spec/CourseOutline_spec.js +++ b/openedx/features/course_experience/static/course_experience/js/spec/CourseOutline_spec.js @@ -1,10 +1,10 @@ /* globals Logger, loadFixtures */ -// import constants from 'edx-ui-toolkit/src/js/utils/constants'; +import { keys } from 'edx-ui-toolkit/src/js/utils/constants'; import { CourseOutline } from '../CourseOutline'; -describe('Course outline factory', () => { +describe('Course Outline factory', () => { let outline; // eslint-disable-line no-unused-vars // Our block IDs are invalid DOM selectors unless we first escape `:`, `+` and `@` @@ -40,7 +40,7 @@ describe('Course outline factory', () => { const current = document.querySelector(outlineIds.homeworkLabsAndDemos); const destination = document.querySelector(outlineIds.homeworkEssays); - triggerKeyListener(current, destination, 'ArrowDown'); // @TODO: Get these from the UI Toolkit + triggerKeyListener(current, destination, keys.down); expect(destination.focus).toHaveBeenCalled(); }); @@ -49,7 +49,7 @@ describe('Course outline factory', () => { const current = document.querySelector(outlineIds.exampleWeek3BeSocial); const destination = document.querySelector(`${outlineIds.exampleWeek3BeSocial} > ol`); - triggerKeyListener(current, destination, 'ArrowDown'); // @TODO: Get these from the UI Toolkit + triggerKeyListener(current, destination, keys.down); expect(destination.focus).toHaveBeenCalled(); }); @@ -58,7 +58,7 @@ describe('Course outline factory', () => { const current = document.querySelector(outlineIds.homeworkEssays); const destination = document.querySelector(outlineIds.exampleWeek3BeSocial); - triggerKeyListener(current, destination, 'ArrowDown'); // @TODO: Get these from the UI Toolkit + triggerKeyListener(current, destination, keys.down); expect(destination.focus).toHaveBeenCalled(); }); @@ -69,7 +69,7 @@ describe('Course outline factory', () => { const current = document.querySelector(outlineIds.homeworkEssays); const destination = document.querySelector(outlineIds.homeworkLabsAndDemos); - triggerKeyListener(current, destination, 'ArrowUp'); // @TODO: Get these from the UI Toolkit + triggerKeyListener(current, destination, keys.up); expect(destination.focus).toHaveBeenCalled(); }); @@ -78,7 +78,7 @@ describe('Course outline factory', () => { const current = document.querySelector(outlineIds.lesson3BeSocial); const destination = document.querySelector(`${outlineIds.exampleWeek3BeSocial} > ol`); - triggerKeyListener(current, destination, 'ArrowUp'); // @TODO: Get these from the UI Toolkit + triggerKeyListener(current, destination, keys.up); expect(destination.focus).toHaveBeenCalled(); }); @@ -87,7 +87,7 @@ describe('Course outline factory', () => { const current = document.querySelector(outlineIds.exampleWeek3BeSocial); const destination = document.querySelector(outlineIds.homeworkEssays); - triggerKeyListener(current, destination, 'ArrowUp'); // @TODO: Get these from the UI Toolkit + triggerKeyListener(current, destination, keys.up); expect(destination.focus).toHaveBeenCalled(); }); diff --git a/openedx/features/course_experience/templates/course_experience/course-outline-fragment.html b/openedx/features/course_experience/templates/course_experience/course-outline-fragment.html index db33eaae4c..21bfa195b8 100644 --- a/openedx/features/course_experience/templates/course_experience/course-outline-fragment.html +++ b/openedx/features/course_experience/templates/course_experience/course-outline-fragment.html @@ -161,5 +161,5 @@ from openedx.core.djangolib.markup import HTML, Text <%static:webpack entry="CourseOutline"> - new CourseOutline(); + new CourseOutline('.block-tree'); diff --git a/package.json b/package.json index b7ffea7ac6..c8e898d727 100644 --- a/package.json +++ b/package.json @@ -2,11 +2,14 @@ "name": "edx", "version": "0.1.0", "dependencies": { + "babel-core": "^6.23.0", + "babel-loader": "^6.4.0", + "babel-preset-env": "^1.2.1", "backbone": "~1.3.2", "backbone.paginator": "~2.0.3", "coffee-script": "1.6.1", "edx-pattern-library": "0.18.1", - "edx-ui-toolkit": "1.5.1", + "edx-ui-toolkit": "1.5.2", "hls.js": "0.7.2", "jquery": "~2.2.0", "jquery-migrate": "^1.4.1", @@ -17,12 +20,11 @@ "requirejs": "~2.3.2", "uglify-js": "2.7.0", "underscore": "~1.8.3", - "underscore.string": "~3.3.4" + "underscore.string": "~3.3.4", + "webpack": "^2.2.1", + "webpack-bundle-tracker": "^0.2.0" }, "devDependencies": { - "babel-core": "^6.23.0", - "babel-loader": "^6.4.0", - "babel-preset-env": "^1.2.1", "edx-custom-a11y-rules": "0.1.3", "eslint-config-edx": "^2.0.1", "eslint-config-edx-es5": "^2.0.0", @@ -43,8 +45,6 @@ "pa11y-reporter-json-oldnode": "1.0.0", "plato": "1.2.2", "sinon": "^1.17.7", - "squirejs": "^0.1.0", - "webpack": "^2.2.1", - "webpack-bundle-tracker": "^0.2.0" + "squirejs": "^0.1.0" } } diff --git a/pavelib/assets.py b/pavelib/assets.py index af77944767..612112c9fe 100644 --- a/pavelib/assets.py +++ b/pavelib/assets.py @@ -704,14 +704,17 @@ def execute_compile_sass(args): ) -@task -@no_help -def execute_webpack(): - sh(cmd("$(npm bin)/webpack")) +def execute_webpack(prod): + sh(cmd("NODE_ENV={node_env} STATIC_ROOT={static_root} $(npm bin)/webpack".format( + node_env="production" if prod else "development", + static_root=Env.get_django_setting("STATIC_ROOT", "lms") + ))) def execute_webpack_watch(): - run_background_process("$(npm bin)/webpack --watch --watch-poll=200") + run_background_process("STATIC_ROOT={static_root} $(npm bin)/webpack --watch --watch-poll=200".format( + static_root=Env.get_django_setting("STATIC_ROOT", "lms") + )) def get_parsed_option(command_opts, opt_key, default=None): @@ -845,7 +848,7 @@ def update_assets(args): process_xmodule_assets() process_npm_assets() compile_coffeescript() - execute_webpack() + execute_webpack(prod=(args.settings != "devstack")) # Compile sass for themes and system execute_compile_sass(args) diff --git a/pavelib/paver_tests/test_servers.py b/pavelib/paver_tests/test_servers.py index e06fe50db9..91d7126e4f 100644 --- a/pavelib/paver_tests/test_servers.py +++ b/pavelib/paver_tests/test_servers.py @@ -4,6 +4,7 @@ import ddt from paver.easy import call_task from .utils import PaverTestCase +from ..utils.envs import Env EXPECTED_COFFEE_COMMAND = ( u"node_modules/.bin/coffee --compile `find {platform_root}/lms " @@ -40,8 +41,11 @@ EXPECTED_RUN_SERVER_COMMAND = ( EXPECTED_INDEX_COURSE_COMMAND = ( u"python manage.py {system} --settings={settings} reindex_course --setup" ) +EXPECTED_PRINT_SETTINGS_COMMAND = ( + u"python manage.py {system} --settings=aws print_settings STATIC_ROOT --format=value 2>/dev/null" +) EXPECTED_WEBPACK_COMMAND = ( - u"$(npm bin)/webpack" + u"NODE_ENV={node_env} STATIC_ROOT={static_root} $(npm bin)/webpack" ) @@ -236,7 +240,11 @@ class TestPaverServerTasks(PaverTestCase): expected_messages.append(u"xmodule_assets common/static/xmodule") expected_messages.append(u"install npm_assets") expected_messages.append(EXPECTED_COFFEE_COMMAND.format(platform_root=self.platform_root)) - expected_messages.append(EXPECTED_WEBPACK_COMMAND) + expected_messages.append(EXPECTED_PRINT_SETTINGS_COMMAND.format(system="lms")) + expected_messages.append(EXPECTED_WEBPACK_COMMAND.format( + node_env="production" if expected_asset_settings != "devstack" else "development", + static_root=None + )) expected_messages.extend(self.expected_sass_commands(system=system, asset_settings=expected_asset_settings)) if expected_collect_static: expected_messages.append(EXPECTED_COLLECT_STATIC_COMMAND.format( @@ -274,7 +282,11 @@ class TestPaverServerTasks(PaverTestCase): expected_messages.append(u"xmodule_assets common/static/xmodule") expected_messages.append(u"install npm_assets") expected_messages.append(EXPECTED_COFFEE_COMMAND.format(platform_root=self.platform_root)) - expected_messages.append(EXPECTED_WEBPACK_COMMAND) + expected_messages.append(EXPECTED_PRINT_SETTINGS_COMMAND.format(system="lms")) + expected_messages.append(EXPECTED_WEBPACK_COMMAND.format( + node_env="production" if expected_asset_settings != "devstack" else "development", + static_root=None + )) expected_messages.extend(self.expected_sass_commands(asset_settings=expected_asset_settings)) if expected_collect_static: expected_messages.append(EXPECTED_COLLECT_STATIC_COMMAND.format( diff --git a/pavelib/utils/envs.py b/pavelib/utils/envs.py index 15e5e997a2..8156adae6b 100644 --- a/pavelib/utils/envs.py +++ b/pavelib/utils/envs.py @@ -7,6 +7,8 @@ import sys import json from lazy import lazy from path import Path as path +from pavelib.utils.cmd import django_cmd +from paver.easy import sh import memcache @@ -171,6 +173,23 @@ class Env(object): else: SERVICE_VARIANT = 'lms' + @classmethod + def get_django_setting(self, django_setting, system): + """ + Interrogate Django environment for specific settings values + """ + value = sh( + django_cmd( + system, + os.environ.get("EDX_PLATFORM_SETTINGS", "aws"), + "print_settings {django_setting} --format=value 2>/dev/null".format( + django_setting=django_setting + ) + ), + capture=True + ) + return unicode(value).strip() + @lazy def env_tokens(self): """ diff --git a/scripts/jenkins-common.sh b/scripts/jenkins-common.sh index f51bf02159..20e81b7a40 100644 --- a/scripts/jenkins-common.sh +++ b/scripts/jenkins-common.sh @@ -64,6 +64,9 @@ echo "npm version is `npm --version`" echo "--> Cleaning npm cache" npm cache clean +echo "setting variables that paver will use to get Django Settings" +export EDX_PLATFORM_SETTINGS=test_static_optimized + # Log any paver or ansible command timing TIMESTAMP=$(date +%s) export PAVER_TIMER_LOG="test_root/log/timing.paver.$TIMESTAMP.log" diff --git a/webpack.config.js b/webpack.config.js index 1e90a7522e..7019824b77 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -1,3 +1,7 @@ +/* eslint-env node */ + +'use strict'; + var path = require('path'); var webpack = require('webpack'); var BundleTracker = require('webpack-bundle-tracker'); @@ -13,7 +17,7 @@ var wpconfig = { output: { path: path.resolve(__dirname, 'common/static/bundles'), - filename: '[name]-[hash].js', + filename: isProd ? '[name].[chunkhash].js' : '[name].js', libraryTarget: 'window' }, @@ -29,7 +33,8 @@ var wpconfig = { debug: !isProd }), new BundleTracker({ - filename: './webpack-stats.json' + path: process.env.STATIC_ROOT, + filename: 'webpack-stats.json' }) ],