Use Rollup #4301

gkatsev · 2017-04-21T21:08:06Z

Based partially on https://github.com/videojs/video.js/compare/master...Rich-Harris:rollup?expand=1
Currently, exports UMD, minified UMD, CJS, and ES rolluped files as well as equivalent formats for the novtt file.
Still needs to be updated to work with the tests, which could be an issue, potentially, because of proxyquireify. Maybe tests will stay as they are.

The build is currently failing because the only babelrc is told to not compile module syntax and that is causing grunt and things to fail.

gkatsev · 2017-04-21T21:12:00Z

Here's the assets output:

┌───────────────────────────────┬───────────┬───────────┐
│ filename                      │      size │   gzipped │
├───────────────────────────────┼───────────┼───────────┤
│ alt/video-js-cdn.css          │  53.12 KB │  13.99 KB │
├───────────────────────────────┼───────────┼───────────┤
│ alt/video-js-cdn.min.css      │   45.6 KB │  13.69 KB │
├───────────────────────────────┼───────────┼───────────┤
│ alt/video.novtt.js            │ 796.66 KB │ 151.63 KB │
├───────────────────────────────┼───────────┼───────────┤
│ alt/video.novtt.min.js        │ 226.18 KB │  47.63 KB │
├───────────────────────────────┼───────────┼───────────┤
│ alt/video.novtt.rollup.cjs.js │  604.6 KB │ 134.85 KB │
├───────────────────────────────┼───────────┼───────────┤
│ alt/video.novtt.rollup.es.js  │ 604.58 KB │ 134.84 KB │
├───────────────────────────────┼───────────┼───────────┤
│ alt/video.novtt.rollup.js     │ 604.83 KB │ 134.93 KB │
├───────────────────────────────┼───────────┼───────────┤
│ alt/video.novtt.rollup.min.js │ 156.56 KB │  40.42 KB │
├───────────────────────────────┼───────────┼───────────┤
│ video-js.css                  │  53.31 KB │     14 KB │
├───────────────────────────────┼───────────┼───────────┤
│ video-js.min.css              │  45.74 KB │  13.69 KB │
├───────────────────────────────┼───────────┼───────────┤
│ video-js.swf                  │  16.72 KB │  16.74 KB │
├───────────────────────────────┼───────────┼───────────┤
│ video.js                      │ 853.01 KB │ 165.83 KB │
├───────────────────────────────┼───────────┼───────────┤
│ video.min.js                  │ 245.82 KB │  53.97 KB │
├───────────────────────────────┼───────────┼───────────┤
│ video.rollup.cjs.js           │  732.1 KB │ 154.19 KB │
├───────────────────────────────┼───────────┼───────────┤
│ video.rollup.es.js            │ 732.08 KB │ 154.18 KB │
├───────────────────────────────┼───────────┼───────────┤
│ video.rollup.js               │ 732.33 KB │ 154.26 KB │
├───────────────────────────────┼───────────┼───────────┤
│ video.rollup.min.js           │ 213.41 KB │  49.48 KB │
└───────────────────────────────┴───────────┴───────────┘

all the files that rollup currently generates have rollup in the name and generally correspond to the same files that don't have rollup.

One thing I just realized is that the cjs builds shouldn't be running with node-resolve and commonjs plugins so bundlers will pick those up. The ES one should potentially do that as well.

gkatsev · 2017-04-27T21:21:23Z

Will require this change in vttjs: videojs/vtt.js#14

misteroneill · 2017-05-08T17:06:03Z

build/rollup.js

+
+minifiedUmd.options.plugins.splice(4, 0, uglify({
+  preserveComments: 'some',
+  srewIE8: false,


Appears to be a typo.

This reverts commit b8892b3.

This reverts commit 78d13c4.

gkatsev · 2017-05-17T18:26:07Z

package.json

@@ -128,13 +150,14 @@
    "videojs-doc-generator": "0.0.1",
    "videojs-flash": "^1.1.0",
    "videojs-standard": "^6.0.1",
-    "webpack": "^2.3.0"
+    "webpack": "^1.15.0"


webpack 2 doesn't support IE8: webpack/webpack#3070

gkatsev · 2017-05-17T20:58:09Z

I believe this is good to go now. One way to test and verify this is to clone this branch, npm install and run npm run build. Then, clone videojs-bundler-sample and in it run npm install /path/to/videojs followed by npm start. Then, if you go to the index page in the bundler repo in the browser, you'll be able to see it with all the various types of bundlers. Doing npm install with a path over linking is necessary because otherwise babel ends up breaking for some of the builds.

Updated and needs a re-review and test

gkatsev · 2017-05-17T21:01:15Z

The current output of the assets script:

┌──────────────────────────┬───────────┬───────────┐
│ filename                 │      size │   gzipped │
├──────────────────────────┼───────────┼───────────┤
│ alt/video-js-cdn.css     │  53.12 KB │  13.98 KB │
├──────────────────────────┼───────────┼───────────┤
│ alt/video-js-cdn.min.css │   45.6 KB │  13.69 KB │
├──────────────────────────┼───────────┼───────────┤
│ alt/video.novtt.js       │ 611.79 KB │ 136.34 KB │
├──────────────────────────┼───────────┼───────────┤
│ alt/video.novtt.min.js   │ 158.16 KB │  40.79 KB │
├──────────────────────────┼───────────┼───────────┤
│ video-js.css             │  53.31 KB │     14 KB │
├──────────────────────────┼───────────┼───────────┤
│ video-js.min.css         │  45.74 KB │  13.69 KB │
├──────────────────────────┼───────────┼───────────┤
│ video-js.swf             │  16.91 KB │  16.93 KB │
├──────────────────────────┼───────────┼───────────┤
│ video.cjs.js             │ 600.68 KB │ 133.11 KB │
├──────────────────────────┼───────────┼───────────┤
│ video.es.js              │ 600.43 KB │ 133.05 KB │
├──────────────────────────┼───────────┼───────────┤
│ video.js                 │ 664.96 KB │ 150.79 KB │
├──────────────────────────┼───────────┼───────────┤
│ video.min.js             │ 177.58 KB │  47.35 KB │
└──────────────────────────┴───────────┴───────────┘

gkatsev · 2017-05-17T21:14:47Z

Here's a merged table:

┌──────────────────────────┬─────────────────┬─────────────┬────────────────────┬────────────────┐
│ filename                 │ size browserify │ size rollup │ gzipped browserify │ gzipped rollup │
├──────────────────────────┼─────────────────┼─────────────┼────────────────────┼────────────────┤
│ alt/video-js-cdn.css     │        53.12 KB │    53.12 KB │           13.99 KB │       13.98 KB │
├──────────────────────────┼─────────────────┼─────────────┼────────────────────┼────────────────┤
│ alt/video-js-cdn.min.css │         45.6 KB │     45.6 KB │           13.69 KB │       13.69 KB │
├──────────────────────────┼─────────────────┼─────────────┼────────────────────┼────────────────┤
│ alt/video.novtt.js       │       796.66 KB │   611.79 KB │          151.63 KB │      136.34 KB │
├──────────────────────────┼─────────────────┼─────────────┼────────────────────┼────────────────┤
│ alt/video.novtt.min.js   │       226.18 KB │   158.16 KB │           47.63 KB │       40.79 KB │
├──────────────────────────┼─────────────────┤─────────────┼────────────────────┼────────────────┤
│ video-js.css             │        53.31 KB │    53.31 KB │              14 KB │          14 KB │
├──────────────────────────┼─────────────────┤─────────────┼────────────────────┼────────────────┤
│ video-js.min.css         │        45.74 KB │    45.74 KB │           13.69 KB │       13.69 KB │
├──────────────────────────┼─────────────────┤─────────────┼────────────────────┼────────────────┤
│ video-js.swf             │        16.72 KB │    16.91 KB │           16.74 KB │       16.93 KB │
├──────────────────────────┼─────────────────┤─────────────┼────────────────────┼────────────────┤
│ video.cjs.js             │ N/A             │   600.68 KB │ N/A                │      133.11 KB │
├──────────────────────────┼─────────────────┼─────────────┼────────────────────┼────────────────┤
│ video.es.js              │ N/A             │   600.43 KB │ N/A                │      133.05 KB │
├──────────────────────────┼─────────────────┼─────────────┼────────────────────┼────────────────┤
│ video.js                 │       853.01 KB │   664.96 KB │          165.83 KB │      150.79 KB │
├──────────────────────────┼─────────────────┤─────────────┼────────────────────┼────────────────┤
│ video.min.js             │       245.82 KB │   177.58 KB │           53.97 KB │       47.35 KB │
└──────────────────────────┴─────────────────┴─────────────┴────────────────────┴────────────────┘

misteroneill

Looks good to me. Works fine. I had one tiny comment/suggestion. Do with it what you will.

misteroneill · 2017-05-18T17:59:43Z

build/rollup.js

+      }
+      if (warning.code === 'UNRESOLVED_IMPORT') {
+        return;
+      }


Maybe these two blocks should be combined.

gkatsev added 5 commits April 21, 2017 14:10

print out filesize report for dist files, include gzipped sizes

bc4abdb

npm script for assets

44cd535

use rollup to generate main, module, and umd builds of video.js

c659cb6

build novtt file with rollup

bd7971b

update rollup scripts

7813801

gkatsev added 10 commits April 26, 2017 15:23

run assets script after a successful travis run

3cf06a1

use standard babelrc, rollup configured babel manually

7625335

don't use rollup config anymore

a1af6ac

update package.json and lint rollup file

2ae1284

Switch to rollup API for building and watching

b7019a8

fix npm scripts

8ff9812

Turn on legacy mode and pass options to uglify

e8182da

Output to dist directly

218bdc7

make sure we keep the banner

4b5c584

fix eslint

8ef0404

gkatsev added 4 commits April 27, 2017 17:23

prepare for vttjs update

d26a837

update filepaths

8a78284

update vttjs

e1463ea

WIP build

64ffe6d

misteroneill previously requested changes May 8, 2017

View reviewed changes

gkatsev added 7 commits May 16, 2017 13:47

upadte vttjs url

1482381

update grunt build

5b1f346

new js location is at dist/

b465368

use unminified ie8 file in karma

1df1504

Merge branch 'master' into rollup

51cacbc

Merge branch 'master' into rollup

9302c90

add a no-progress option to rollup

1ae752a

gkatsev added 8 commits May 16, 2017 16:50

fix package.json version for rollup

a7bb0a6

log npm ls after install

548004f

use after_failure instead

c607a4e

use exact karma versions

78d13c4

grab ie8 shim file from node_modules

b8892b3

Revert "grab ie8 shim file from node_modules"

71ceacc

This reverts commit b8892b3.

Revert "use exact karma versions"

9873f6d

This reverts commit 78d13c4.

downgrade webpack to 1.x because 2.x doesn't support IE8

41826cf

gkatsev commented May 17, 2017

View reviewed changes

typo screwIE8

0be795a

gkatsev changed the title ~~WIP: Use Rollup~~ Use Rollup May 17, 2017

misteroneill approved these changes May 18, 2017

View reviewed changes

build/rollup.js Outdated

}

if (warning.code === 'UNRESOLVED_IMPORT') {

return;

}

Copy link

Member

misteroneill May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe these two blocks should be combined.

consolidate ifs

8b17a5f

gkatsev merged commit c31836c into master May 25, 2017

gkatsev deleted the rollup branch May 25, 2017 23:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Rollup #4301

Use Rollup #4301

gkatsev commented Apr 21, 2017

gkatsev commented Apr 21, 2017

gkatsev commented Apr 27, 2017

misteroneill May 8, 2017

gkatsev May 8, 2017

gkatsev May 17, 2017

gkatsev commented May 17, 2017

gkatsev commented May 17, 2017

gkatsev commented May 17, 2017

misteroneill left a comment

misteroneill May 18, 2017

Use Rollup #4301

Use Rollup #4301

Conversation

gkatsev commented Apr 21, 2017

gkatsev commented Apr 21, 2017

gkatsev commented Apr 27, 2017

misteroneill May 8, 2017

Choose a reason for hiding this comment

gkatsev May 8, 2017

Choose a reason for hiding this comment

gkatsev May 17, 2017

Choose a reason for hiding this comment

gkatsev commented May 17, 2017

gkatsev commented May 17, 2017

gkatsev commented May 17, 2017

misteroneill left a comment

Choose a reason for hiding this comment

misteroneill May 18, 2017

Choose a reason for hiding this comment