Use webpack to build files and webpack-dev-server for testing by ylemkimon · Pull Request #1068 · KaTeX/KaTeX

ylemkimon · 2018-01-16T09:30:14Z

Fixes #1063
Fixes partially #929 (webpack-dev-server reloads when katex.less is changed)
Fixes #692
Fixes #983

Old	New
`browserify`(`make *.js`)	`webpack`
browserify transform `babelify`	webpack `babel-loader`
`UglifyJS` CLI(`make *.min.js`)	webpack `UglifyJsPlugin`
`Less` CLI, copy .css(`make *.css`)	webpack `less-loader`, `css-loader`, and `ExtractTextPlugin`
`clean-css` CLI(`make *.min.css`)	`cssnano` included in webpack `css-loader`
fonts(`make build/fonts`)	webpack `file-loader`
contrib(`make contrib` and Makefiles for each contrib)	webpack entry points in `webpack.common.js`[1]
`make build/katex.js build/katex.min.js build/katex.css build/katex.min.css build/fonts build/contrib`	Depends on `make` target `webpack`, which does `npm run build`(`webpack`)
`make` recompile	`webpack --watch`(`npm run watch`)
Local testing using `server.js`(Removed in #902), `webpack-dev-server`(`webpackDevServer.js`)	Updated `webpack-dev-server` configuration(renamed to `webpack.dev.js`), updated documentations
Build stylesheets before starting dev server(npm script `prestart`)	Load(inline) stylesheets using webpack `style-loader`[2]
Screenshotter using `server.js`	Use `webpack-dev-server`
Export CommonJS module	Export ES6 module[3], expose default export

Possible Breaking Changes:

Ref	Changes	Impact	Migration Path
[1]	Removed make target `contrib`	`make contrib`	`make build/contrib` or `npm run build`(`webpack`)
[2]	Removed npm script `build-css`	`npm run build-css`	`make [.css filename]` or `npm run build`(`webpack`)
[3]	Switched to ES6 module export	Directly requiring `./katex.js` (`require('./katex.js)`)	Use `default` property (`require('./katex.js).default`)

I'm not sure there are use cases of above cases.

Created a webpack entry point for KaTeX, which imports katex.less. As flow[1] and jest[2] doesn't support CSS modules natively, a separate entry point is used and it is not flowtyped. [1] https://gist.github.com/lambdahands/d19e0da96285b749f0ef [2] https://facebook.github.io/jest/docs/en/webpack.html

* Made webpack.config.js export valid webpack configuration * Use: browserify -> webpack babelify -> babel-loader UglifyJS CLI -> UglifyJsPlugin Less CLI -> less-loader cleancss -> cssnano in css-loader build/fonts -> file-loader * Inline CSS(Less) using style-loader and export them using ExtractTextPlugin * Add `watch` npm script calling `webpack --watch`

* Made webpackDevServer export a valid webpack configuration * Compile Less and inline CSS using less-loader and style-loader * Instead of copying files serve files from /static and use file-loader * Remove old server.js and its dependencies

khanbot · 2018-01-16T09:30:25Z

CLA signature looks good 👍

+ Moved common configurations to webpack.common.js

ylemkimon · 2018-01-16T17:37:18Z

webpackDevServer.js

+// @flow
+const { targets, createConfig } = require('./webpack.common');
+const path = require('path');
 const PORT = 7936;


Is there any specific reason using the port 7936?

We wanted to avoid a collision with other services that are usually running in the normal dev environment at Khan Academy.

ylemkimon · 2018-01-16T17:43:33Z

contrib/mathtex-script-type/Makefile

-$(BUILDDIR)/contrib/mathtex-script-type.min.js: $(BUILDDIR)/mathtex-script-type.js
-	$(UGLIFYJS) < $< > $@
-
-$(BUILDDIR)/mathtex-script-type.js: mathtex-script-type.js


Is outputting mathtex-script-type.js to $(BUILDDIR), not $(BUILDDIR)/contrib, and not including in the distribution archive an intended behavior?

Good catch.

ylemkimon · 2018-01-16T17:45:04Z

contrib/copy-tex/Makefile

-	$(UGLIFYJS) < $< > $@
-
-$(BUILDDIR)/contrib/copy-tex.js: copy-tex.js
-	$(BROWSERIFY) -t [ babelify ] $< --standalone renderMathInElement > $@


Is exporting the copy-tex module to renderMathInElement, a name also used by auto-render, an intended behavior?

Thanks for catching this. I guess nobody's been using this as a standalone script.

k4b7

Thanks for the PR. I've used webpack before, but haven't used it for building styles so I have a lot of questions about that.

k4b7 · 2018-01-17T02:53:20Z

webpack.common.js

+                    use: [{
+                        loader: 'file-loader',
+                        options: {
+                            name: 'fonts/[name].[ext]',


This just copies the files into the fonts folder?

I've never handled fonts like this before. If we didn't do this we'd need a separate way to serve font files when using the dev server correct?

@kevinbarabash It loads included font files in the stylesheet(fonts.less) and outputs to fonts/.

For this to get used, you'd have to import the font into the dependency tree, right? Is that being done?

@rrandallcainc Yes. Current dependency tree is: katex.wepack.js imports static/katex.less, static/katex.less imports static/fonts.less, and fonts.less imports(using @font-face) font files.

k4b7 · 2018-01-17T02:56:11Z

webpack.common.js

+    name: string, // the name of output JS/CSS
+    entry: string, // the path to the entry point
+    library?: string // the name of the exported module
+|};


Using comments with flow to avoid having to transpile?

@kevinbarabash Yes 😄

k4b7 · 2018-01-17T03:00:42Z

webpack.common.js

+        config.plugins.push(new UglifyJsPlugin({
+            uglifyOptions: {
+                output: {
+                    ascii_only: true,


The Makefile used --mangle --beautify ascii_only=true,beautify=false. Does UglifyJsPlugin support the same options?

@kevinbarabash Yes. mangle is enabled by default and beautify=false was necessary in order to pass ascii_only=true to the CLI(mishoo/UglifyJS#54).

k4b7 · 2018-01-17T03:01:00Z

webpack.common.js

+                disable: dev,
+            }),
+        ],
+        devtool: dev && 'inline-source-map',


k4b7 · 2018-01-17T03:05:58Z

webpack.common.js

+    const cssLoader = {
+        loader: 'css-loader',
+        options: {
+            minimize, // cssnano


What's cssnano?

@kevinbarabash cssnano is a CSS minifier equivalent to clean-css and it is included in the css-loader, which can be enabled by passing minimize: true to its options.

k4b7 · 2018-01-17T03:27:33Z

Makefile

 	mkdir -p build
-	$(CLEANCSS) -o $@ $<
+	mkdir -p build/contrib
+	mkdir -p build/fonts


Doesn't webpack create the directories it needs if they don't exist?

@kevinbarabash Yes, it does. Will remove them.

k4b7 · 2018-01-17T03:27:59Z

Makefile

+	@# Since everything in build/contrib and build/fonts is put in the built files,
+	@# make sure there's nothing in there we don't want.
+	rm -rf build/contrib/*
+	rm -rf build/fonts/*


I don't if we should just do rm -rf build?

@kevinbarabash I think preserving other files in build/ is no longer necessary as dev server doesn't generate or use files in build/. Will change to rm -rf build/*

k4b7 · 2018-01-17T03:28:29Z

contrib/auto-render/README.md

+(To run this example from a clone of the repository, run `npm start`
 in the root KaTeX directory, and then visit
-http://0.0.0.0:7936/contrib/auto-render/index.html
+http://localhost:7936/contrib/auto-render/index.html


Thanks for updating the docs.

k4b7 · 2018-01-17T03:28:58Z

contrib/auto-render/index.html

    <meta charset="UTF-8">
    <title>Auto-render test</title>
    <script src="/katex.js" type="text/javascript"></script>
-    <link href="/katex.css" rel="stylesheet" type="text/css">


Don't we still need katex.css?

k4b7 · 2018-01-17T03:29:28Z

contrib/copy-tex/index.html

    <title>Copy-tex test</title>
    <script src="/katex.js" type="text/javascript"></script>
-    <link href="/katex.css" rel="stylesheet" type="text/css">
-    <link href="./copy-tex.css" rel="stylesheet" type="text/css">


Why no .css files?

I believe webpack will now inject these with either a script tag (extract plugin) or directly into the JS.

k4b7 · 2018-01-17T03:37:36Z

Fixes partially #929 (webpack-dev-server reloads when katex.less is changed)

Why is it a partial fix?

k4b7 · 2018-01-17T03:40:36Z

I restarted the travis-ci build b/c I was really confused by the output of the previous run. It should be running npm test, but I couldn't find any of the usual output that I expect from that command. I ran npm test locally on your branch and it seems to work fine.

ylemkimon · 2018-01-17T04:35:22Z

@kevinbarabash Currently Travis seems to be not working and backlogged.

ylemkimon · 2018-01-17T04:49:21Z

@kevinbarabash

Why is it a partial fix?

As static/main.js is not handled by webpack, it's not watched and doesn't reload if changed.

to be consistent, avoid confusion with webpack-dev-server and follow webpack configuration naming convention.

+ Add comments regarding function arguments

Fixes #692.

a0d8b33

This reverts commit f6b5091.

k4b7 · 2018-01-17T16:52:46Z

contrib/auto-render/auto-render.js

 /* eslint no-console:0 */
-/* global katex */

+import katex from "katex";


Won't this result in the bundle that's built for auto-render.js also containing all of KaTeX?

k4b7 · 2018-01-17T16:53:13Z

contrib/mathtex-script-type/mathtex-script-type.js

@@ -1,24 +1,22 @@
-/* global katex */
+import katex from "katex";


Same concern as in auto-render.js.

@kevinbarabash The module katex is declared as externals in the webpack configuration. As a result, webpack doesn't bundle the KaTeX, but tries to require the module by require("katex") in CJS, define(["katex"], ...) in AMD, and using global variable katex(old behavior) otherwise. Basically, it adds support for module loaders.

Oh wow. That's super cool!

ry-randall

This is really exciting to see, thanks for opening!

ry-randall · 2018-01-17T17:05:01Z

webpack.common.js

+                    use: [{
+                        loader: 'file-loader',
+                        options: {
+                            name: 'fonts/[name].[ext]',


For this to get used, you'd have to import the font into the dependency tree, right? Is that being done?

ry-randall · 2018-01-17T17:06:46Z

webpack.config.js

@@ -1,61 +1,7 @@
-const path = require('path');
+// @flow
+const { targets, createConfig } = require('./webpack.common');


Would we want to rename this to webpack.prod.js?

@rrandallcainc As webpack's default configuration filename is webpack.config.js, if webpack is called without any arguments, production build will be run.

ry-randall · 2018-01-17T17:10:49Z

contrib/copy-tex/index.html

    <title>Copy-tex test</title>
    <script src="/katex.js" type="text/javascript"></script>
-    <link href="/katex.css" rel="stylesheet" type="text/css">
-    <link href="./copy-tex.css" rel="stylesheet" type="text/css">


I believe webpack will now inject these with either a script tag (extract plugin) or directly into the JS.

ry-randall · 2018-01-17T17:11:50Z

dockers/Screenshotter/screenshotter.js

    // Now we need to find an IP the container can connect to.
    // First, install a server component to get notified of successful connects
-    app.get("/ss-connect.js", function(req, res, next) {
+    devServer.app.get("/ss-connect.js", function(req, res, next) {


ry-randall · 2018-01-17T17:12:14Z

katex.js

 };

-module.exports = {
+export default {


Is this a breaking change for CJS users?

@rrandallcainc For dist/katex.js, which is distribution file and entry point of katex module, as webpack exports an UMD module and default export is exposed, I think there'll be no breaking changes.

(ii) katex.js: transpilers and/or bundlers uses interopDefault, which imports default export when importing ES6 module
I think there'll be no breaking changes.

@rrandallcainc I'm don't know there is a use case of directly requiring katex.js (from the root), e.g., const katex = require('./node_modules/katex/katex.js);, as it needs transpiling, but in that case, the user has to use .default, e.g., const katex = require('./node_modules/katex/katex.js).default;.

Hmm, but I still think katex.js serves as the entry point, right? Even though this will be wrapped in a UMD, I think it's going to be a breaking change when requiring the built artifact. This is due to ES6 modules always being named. When CJS users try and import Katex, they'll need to change their import from require('katex') to require('katex').default.

Perhaps I am missing something though.

@rrandallcainc Webpack has option libraryExport, which configures which module(s) will be exposed via UMD and it is set to default. So requiring katex module(require('katex')) will expose default export and work same.

ry-randall · 2018-01-17T17:12:35Z

server.js

@@ -1,106 +0,0 @@
-/* eslint no-console:0 */


k4b7 · 2018-01-18T05:07:11Z

package.json

  "version": "0.10.0-pre",
  "description": "Fast math typesetting for the web.",
  "main": "dist/katex.js",
-  "browser": "dist/katex.min.js",


Why was this reverted?

@kevinbarabash #1057 is no longer needed, since webpack doesn't use name require and browserify/browserify#374 is not an issue.

…otter

Allows webpack to be run from other directories

ylemkimon · 2018-01-18T09:12:57Z

Updated PR description 😄

Do not transform modules to commonjs in Babel. However Jest doesn't not support ES6 modules, so transfrom modules to commonjs when NODE_ENV is `test`.

ylemkimon · 2018-01-18T15:14:55Z

.babelrc

+    "env": {
+        "test": {
+            "plugins": [
+                "transform-es2015-modules-commonjs"


This is for Jest, as it doesn't support ES6 modules.
In Babel 6, env configurations doesn't overrides configuration, rather are merged to the root configuration(babel/babel#5276).

Could you avoid adding this dependency by adding the .default to your imports instead?

@rrandallcainc Node doesn't support ES6 import/export module syntaxes, so I think adding .default won't help it. Also this plugin is part of babel-preset-es2015, so there won't be any additional dependencies.

k4b7 · 2018-01-18T17:19:17Z

@ylemkimon thanks for updating the comment at the top. I'm pretty busy this week. I'll try to do another pass on the weekend.

Changed version range to include IE-compatible version

ylemkimon · 2018-01-20T07:41:07Z

webpack.common.js

@@ -0,0 +1,121 @@
+// @flow


It seems it's possible to
(i) use ES6 in webpack.config.js by using the extension .babel.js, which transpiles the file before loading
(ii) put all configurations in one file by webpack.config.js exporting a function, which takes environment variables as an argument and returns the webpack configuration

I think having the whole config in a single file and using environment variables to control things might be nice. I'm going land this as is though and any additional work could happen in a followup PR.

k4b7 · 2018-01-22T00:34:08Z

@ylemkimon thanks for listing out the breaking changes. I think the only breaking change that might affect users would be the third one, but my guess would be that most users are using the compiled bundle so this shouldn't be an issue. We can add it to the breaking changes part of the release notes when we do a release.

From the "old/new" section:

Export CommonJS module | Export ES6 module[3], expose default export

This sounds scarier than it is b/c the bundle is built in such a way that it exposes a commonjs interface as well as a katex global as per the discussion in the comments. Could you update your comment to make it more explicit that the interface the published module exports remains unchanged?

k4b7

LGTM. I'm really happy that we have a more consistent build system. Also, looking forward to being able to experiment with webpack plugins such as ModuleConcatenationPlugin.

edemaine · 2018-01-22T06:33:35Z

Sounds great! Just checking: Does require('katex') still work on Node as it did? We'll definitely want to do another beta (not final) release with this new system.

k4b7 · 2018-01-22T12:47:48Z

I did the following as a quick check:

npm run build
cd build
node
const katex = require('katex') // this returned the katex object as expected

k4b7 · 2018-01-22T19:05:30Z

@davidflanagan just let me know that this change "shaved 40Kb off of katex.min.js!"

ry-randall

Nice work 👍

ylemkimon added 3 commits January 16, 2018 17:43

Improve local testing(webpack-dev-server)

78eab71

* Made webpackDevServer export a valid webpack configuration * Compile Less and inline CSS using less-loader and style-loader * Instead of copying files serve files from /static and use file-loader * Remove old server.js and its dependencies

ylemkimon added 2 commits January 16, 2018 23:25

Use webpack-dev-server in Screenshotter

e19e8f9

Include contrib in webpack-dev-server

b532204

+ Moved common configurations to webpack.common.js

ylemkimon changed the title ~~[WIP]Use webpack to build files and webpack-dev-server for testing~~ Use webpack to build files and webpack-dev-server for testing Jan 16, 2018

ylemkimon commented Jan 16, 2018

View reviewed changes

k4b7 reviewed Jan 17, 2018

View reviewed changes

ylemkimon added 11 commits January 17, 2018 15:39

Rename webpackDevServer.js to webpack.dev...

a0d8b33

to be consistent, avoid confusion with webpack-dev-server and follow webpack configuration naming convention.

Remove unnecessary conditional output.path

d11a8c6

Use map instead of reduce

e26db78

+ Add comments regarding function arguments

Remove unnecessary mkdir and clean build/* before build

ca9e080

Use katex as external dependency instead of global variable in contrib

248671c

Fixes #692.

Unblock codes as they are built as a module

f7258eb

Update package-lock.json

fb6b7dc

Add comments regarding devServer option

d7c2260

Lint renamed webpack.dev.js

b58d413

a0d8b33

Export ES6 module and expose its default export

5ee3894

Revert "Browserify hotfix (#1057)"

dd87528

This reverts commit f6b5091.

k4b7 reviewed Jan 17, 2018

View reviewed changes

ry-randall reviewed Jan 17, 2018

View reviewed changes

k4b7 reviewed Jan 18, 2018

View reviewed changes

ylemkimon added 4 commits January 18, 2018 17:03

Enables colors on the console when running the dev server in Screensh…

927402a

…otter

Add context to webpack configuration

08b482c

Allows webpack to be run from other directories

Move rm -rf build/* to npm scripts

588daf0

Check dependencies before build

34aa2ba

ylemkimon and others added 3 commits January 18, 2018 18:55

Merge branch 'master' into webpack

66b6bad

Move UglifyJsPlugin into config creation

88fe36a

Let webpack handle ES6 modules

e4f6561

Do not transform modules to commonjs in Babel. However Jest doesn't not support ES6 modules, so transfrom modules to commonjs when NODE_ENV is `test`.

ylemkimon commented Jan 18, 2018

View reviewed changes

Add documentation on testing in IE 9 and 10 using webpack-dev-server

051c144

Changed version range to include IE-compatible version

ylemkimon mentioned this pull request Jan 18, 2018

[PoC]Plugin architecture using webpack (HTML \class extension) #1074

Closed

4 tasks

ylemkimon commented Jan 20, 2018

View reviewed changes

k4b7 approved these changes Jan 22, 2018

View reviewed changes

k4b7 merged commit 5f32b71 into KaTeX:master Jan 22, 2018

ry-randall reviewed Jan 22, 2018

View reviewed changes

schober-ch mentioned this pull request Jan 26, 2018

Make contrib/auto-render importable #983

Closed

ylemkimon mentioned this pull request Feb 6, 2018

Update NPM scripts and documentation #1146

Merged

ylemkimon deleted the webpack branch May 20, 2018 11:59

		@@ -1,24 +1,22 @@
		/* global katex */
		import katex from "katex";

Uh oh!

Conversation

ylemkimon commented Jan 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Possible Breaking Changes:

Uh oh!

khanbot commented Jan 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ylemkimon Jan 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k4b7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ylemkimon Jan 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k4b7 commented Jan 17, 2018

Uh oh!

k4b7 commented Jan 17, 2018

Uh oh!

ylemkimon commented Jan 17, 2018

Uh oh!

ylemkimon commented Jan 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ylemkimon commented Jan 16, 2018 •

edited

Loading

ylemkimon Jan 16, 2018 •

edited

Loading

ylemkimon Jan 17, 2018 •

edited

Loading

ylemkimon commented Jan 17, 2018 •

edited

Loading

ylemkimon Jan 18, 2018 •

edited

Loading

ylemkimon Jan 18, 2018 •

edited

Loading

ylemkimon Jan 18, 2018 •

edited

Loading