From 41c12c853c1cc754dd67ff81cf5df70751ee3651 Mon Sep 17 00:00:00 2001 From: Alec Merdler Date: Tue, 1 Aug 2017 13:28:24 -0400 Subject: [PATCH] use Webpack code-splitting to dynamically import Highlight.js languages as they are detected by Showdown Markdown extension --- Dockerfile | 9 +++--- endpoints/common.py | 6 ++-- package.json | 2 -- .../markdown-editor.component.spec.ts | 31 ++++++++++++++++++- .../ui/markdown/markdown-editor.component.ts | 12 +++++-- .../directives/ui/markdown/markdown.module.ts | 20 +++++++----- static/js/types/custom.d.ts | 3 ++ static/test/test-index.ts | 2 +- webpack.config.js | 18 +++++------ 9 files changed, 70 insertions(+), 33 deletions(-) create mode 100644 static/js/types/custom.d.ts diff --git a/Dockerfile b/Dockerfile index 0f75ebac4..3f3bc05c0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -20,15 +20,14 @@ RUN virtualenv --distribute venv \ # Install front-end dependencies # JS depedencies -COPY yarn.lock ./ -RUN yarn install --ignore-engines +COPY yarn.lock package.json tsconfig.json webpack.config.js tslint.json ./ +RUN yarn install --ignore-engines # JS compile COPY static static -COPY package.json tsconfig.json webpack.config.js tslint.json ./ RUN yarn build \ - && jpegoptim static/img/**/*.jpg \ - && optipng -clobber -quiet static/img/**/*.png + && jpegoptim static/img/**/*.jpg \ + && optipng -clobber -quiet static/img/**/*.png COPY . . diff --git a/endpoints/common.py b/endpoints/common.py index 26b45848f..1693c5762 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -56,10 +56,10 @@ def common_login(user_uuid, permanent_session=True): return False -def _list_files(path, extension): +def _list_files(path, extension, contains=""): """ Returns a list of all the files with the given extension found under the given path. """ def matches(f): - return os.path.splitext(f)[1] == '.' + extension and f.split(os.path.extsep)[1] != 'spec' + return os.path.splitext(f)[1] == '.' + extension and contains in os.path.splitext(f)[0] def join_path(dp, f): # Remove the static/ prefix. It is added in the template. @@ -74,7 +74,7 @@ def render_page_template(name, route_data=None, **kwargs): library_styles = [] main_styles = [] library_scripts = [] - main_scripts = _list_files('build', 'js') + main_scripts = _list_files('build', 'js', "bundle") use_cdn = app.config.get('USE_CDN', True) if request.args.get('use_cdn') is not None: diff --git a/package.json b/package.json index 7f841224b..df292d270 100644 --- a/package.json +++ b/package.json @@ -53,8 +53,6 @@ "@types/core-js": "^0.9.39", "@types/jasmine": "^2.5.41", "@types/jquery": "^2.0.40", - "@types/react": "0.14.39", - "@types/react-dom": "0.14.17", "@types/showdown": "^1.4.32", "angular-mocks": "1.6.2", "css-loader": "0.25.0", diff --git a/static/js/directives/ui/markdown/markdown-editor.component.spec.ts b/static/js/directives/ui/markdown/markdown-editor.component.spec.ts index 70a3cf7a3..a1a9f32d3 100644 --- a/static/js/directives/ui/markdown/markdown-editor.component.spec.ts +++ b/static/js/directives/ui/markdown/markdown-editor.component.spec.ts @@ -29,6 +29,16 @@ describe("MarkdownEditorComponent", () => { }); }); + describe("ngOnDestroy", () => { + + it("removes 'beforeunload' event listener", () => { + $windowMock.setup(mock => mock.onbeforeunload).is(() => 1); + component.ngOnDestroy(); + + expect($windowMock.Object.onbeforeunload.call(this)).toEqual(null); + }); + }); + describe("changeEditMode", () => { it("sets component's edit mode to given mode", () => { @@ -147,7 +157,15 @@ describe("MarkdownEditorComponent", () => { describe("discardChanges", () => { - it("emits output event with no content", (done) => { + it("prompts user to confirm discarding changes", () => { + const confirmSpy: Spy = $windowMock.setup(mock => mock.confirm).is((message) => false).Spy; + component.discardChanges(); + + expect(confirmSpy.calls.argsFor(0)[0]).toEqual(`Are you sure you want to discard your changes?`); + }); + + it("emits output event with no content if user confirms discarding changes", (done) => { + $windowMock.setup(mock => mock.confirm).is((message) => true); component.discard.subscribe((event: {}) => { expect(event).toEqual({}); done(); @@ -155,5 +173,16 @@ describe("MarkdownEditorComponent", () => { component.discardChanges(); }); + + it("does not emit output event if user declines confirmation of discarding changes", (done) => { + $windowMock.setup(mock => mock.confirm).is((message) => false); + component.discard.subscribe((event: {}) => { + fail(`Should not emit output event`); + done(); + }); + + component.discardChanges(); + done(); + }); }); }); diff --git a/static/js/directives/ui/markdown/markdown-editor.component.ts b/static/js/directives/ui/markdown/markdown-editor.component.ts index 76c3182c2..38f3d098b 100644 --- a/static/js/directives/ui/markdown/markdown-editor.component.ts +++ b/static/js/directives/ui/markdown/markdown-editor.component.ts @@ -1,4 +1,4 @@ -import { Component, Inject, Input, Output, EventEmitter, ViewChild, HostListener } from 'ng-metadata/core'; +import { Component, Inject, Input, Output, EventEmitter, ViewChild, HostListener, OnDestroy } from 'ng-metadata/core'; import { MarkdownSymbol } from '../../../types/common.types'; import { BrowserPlatform } from '../../../constants/platform.constant'; import './markdown-editor.component.css'; @@ -11,7 +11,7 @@ import './markdown-editor.component.css'; selector: 'markdown-editor', templateUrl: '/static/js/directives/ui/markdown/markdown-editor.component.html' }) -export class MarkdownEditorComponent { +export class MarkdownEditorComponent implements OnDestroy { @Input('<') public content: string; @@ -34,6 +34,10 @@ export class MarkdownEditorComponent { return false; } + public ngOnDestroy(): void { + this.$window.onbeforeunload = () => null; + } + public changeEditMode(newMode: EditMode): void { this.editMode = newMode; } @@ -110,7 +114,9 @@ export class MarkdownEditorComponent { } public discardChanges(): void { - this.discard.emit({}); + if (this.$window.confirm(`Are you sure you want to discard your changes?`)) { + this.discard.emit({}); + } } public get currentEditMode(): EditMode { diff --git a/static/js/directives/ui/markdown/markdown.module.ts b/static/js/directives/ui/markdown/markdown.module.ts index 82ce0d560..d1792d283 100644 --- a/static/js/directives/ui/markdown/markdown.module.ts +++ b/static/js/directives/ui/markdown/markdown.module.ts @@ -1,5 +1,5 @@ import { NgModule } from 'ng-metadata/core'; -import { Converter, ConverterOptions } from 'showdown'; +import { Converter } from 'showdown'; import * as showdown from 'showdown'; import { registerLanguage, highlightAuto } from 'highlight.js/lib/highlight'; import 'highlight.js/styles/vs.css'; @@ -10,14 +10,15 @@ const highlightedLanguages: string[] = require('../../../constants/highlighted-l * Dynamically fetch and register a new language with Highlight.js */ export const addHighlightedLanguage = (language: string): Promise<{}> => { - return new Promise((resolve, reject) => { + return new Promise(async(resolve, reject) => { try { - // TODO(alecmerdler): Use `import()` here instead of `require()` - const langModule = require(`highlight.js/lib/languages/${language}`); + // TODO(alecmerdler): Use `import()` here instead of `System.import()` after upgrading to TypeScript 2.4 + const langModule = await System.import(`highlight.js/lib/languages/${language}`); registerLanguage(language, langModule); + console.debug(`Language ${language} registered for syntax highlighting`); resolve(); } catch (error) { - console.log(`Language ${language} not supported for syntax highlighting`); + console.debug(`Language ${language} not supported for syntax highlighting`); reject(error); } }); @@ -25,7 +26,7 @@ export const addHighlightedLanguage = (language: string): Promise<{}> => { /** - * Showdown JS extension for syntax highlighting using Highlight.js + * Showdown JS extension for syntax highlighting using Highlight.js. Will attempt to register detected languages. */ export const showdownHighlight = (): showdown.FilterExtension => { const htmlunencode = (text: string) => { @@ -39,7 +40,10 @@ export const showdownHighlight = (): showdown.FilterExtension => { const right = ''; const flags = 'g'; const replacement = (wholeMatch: string, match: string, leftSide: string, rightSide: string) => { - // TODO(alecmerdler): Call `addHighlightedLanguage` to load new languages that are detected using code-splitting + const language: string = leftSide.slice(leftSide.indexOf('language-') + ('language-').length, + leftSide.indexOf('"', leftSide.indexOf('language-'))); + addHighlightedLanguage(language).catch(error => null); + match = htmlunencode(match); return leftSide + highlightAuto(match).value + rightSide; }; @@ -53,7 +57,7 @@ export const showdownHighlight = (): showdown.FilterExtension => { }; -// Dynamically import syntax-highlighting supported languages +// Import default syntax-highlighting supported languages highlightedLanguages.forEach((langName) => addHighlightedLanguage(langName)); diff --git a/static/js/types/custom.d.ts b/static/js/types/custom.d.ts new file mode 100644 index 000000000..94e6d20d5 --- /dev/null +++ b/static/js/types/custom.d.ts @@ -0,0 +1,3 @@ +declare var System: { + import: (module: string) => Promise; +}; diff --git a/static/test/test-index.ts b/static/test/test-index.ts index a7e4772b0..4592bcf12 100644 --- a/static/test/test-index.ts +++ b/static/test/test-index.ts @@ -1,4 +1,4 @@ -declare var require: any; +declare var require: NodeRequire; // Require all modules ending in ".spec.ts" from the js directory and all subdirectories var testsContext = (require).context("../js", true, /\.spec\.ts$/); diff --git a/webpack.config.js b/webpack.config.js index 9d4d20d3f..35fd72fa0 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -1,13 +1,14 @@ const webpack = require('webpack'); const path = require('path'); -const highlightedLanguages = require('./static/js/constants/highlighted-languages.constant.json'); let config = { entry: "./static/js/main.ts", output: { path: path.resolve(__dirname, "static/build"), - filename: 'quay-frontend.js' + publicPath: "/static/build/", + filename: '[name]-quay-frontend.bundle.js', + chunkFilename: '[name]-quay-frontend.chunk.js' }, resolve: { extensions: [".ts", ".js"], @@ -47,10 +48,6 @@ let config = { angular: "angular", $: "jquery", }), - // Whitelist highlight-supported languages (based on https://bjacobel.com/2016/12/04/highlight-bundle-size/) - new webpack.ContextReplacementPlugin( - /highlight\.js\/lib\/languages$/, - new RegExp(`^./(${highlightedLanguages.join('|')})$`)), ], devtool: "cheap-module-source-map", }; @@ -60,14 +57,15 @@ let config = { * Production settings */ if (process.env.NODE_ENV === 'production') { - config.plugins.push( + config.plugins.concat([ new webpack.optimize.UglifyJsPlugin({ sourceMap: true, // Disable mangle to prevent AngularJS errors mangle: false - }) - ); - config.output.filename = 'quay-frontend-[hash].js'; + }), + new webpack.optimize.CommonsChunkPlugin({name: 'common'}), + ]); + config.output.filename = '[name]-quay-frontend-[hash].bundle.js'; } module.exports = config;