Perf: Save 597 gz bytes through code sharing #637
Conversation
| @@ -2,22 +2,9 @@ | |||
| * @file resolve-url.js - Handling how URLs are resolved and manipulated | |||
| */ | |||
|
|
|||
| import URLToolkit from 'url-toolkit'; | |||
There was a problem hiding this comment.
does URLToolkit need to be a direct dep of VHS anymore?
There was a problem hiding this comment.
It actually isn't anymore, just a dev dependency, as it's used in tests.
There was a problem hiding this comment.
Ah, cool, missed that it was moved over.
|
I wonder if there could be any issues with the stream.js compat across the libs. They mostly look the same but there's some minor changes. |
|
stream.js only had one difference between aes-decrypter, m3u8-parser, and mux.js and it was actually a bug that was fixed in Mux.js. https://github.com/videojs/vhs-utils/blob/master/src/stream.js#L51 |
|
FYI, tests are failing. Also, we'd want to make sure that the deps get non-prerelease releases before we merge this. |
brandonocasey
force-pushed
the
perf/vhs-utils
branch
2 times, most recently
from
986bad6
to
8cd081f
Compare
brandonocasey
changed the title
brandonocasey
force-pushed
the
perf/vhs-utils
branch
from
14938b3
to
f9242c1
Compare
brandonocasey
force-pushed
the
perf/vhs-utils
branch
from
bb017cd
to
847aef4
Compare
brandonocasey
force-pushed
the
perf/vhs-utils
branch
2 times, most recently
from
dc0d8b1
to
b9c1b62
Compare
brandonocasey
force-pushed
the
perf/vhs-utils
branch
from
b9c1b62
to
5d70af5
Compare
| const player = this.player; | ||
| // TODO: why does this make the next test | ||
| // throw an "The operation was aborted." on firefox | ||
| if (!videojs.browser.IS_FIREFOX) { |
There was a problem hiding this comment.
Firefox 69 started throwing an error "The Operation was aborted", and no amount of promise silencing seems to help. The test itself actually passes, but it throws an error that causes a global failure.
There was a problem hiding this comment.
This happens in master too.
There was a problem hiding this comment.
I wonder if this should instead use QUnit.skip? Maybe QUnit[videojs.browser.IS_FIREFOX ? 'skip' : testFn]?
| function(assert) { | ||
| const done = assert.async(); | ||
| let appendsFinished = 0; | ||
| if (!videojs.browser.IS_EDGE) { |
There was a problem hiding this comment.
This test is failing intermittently on edge, even in master, since edge throws on setDuration sometimes.
There was a problem hiding this comment.
Same comment about skipping here.
| const player = this.player; | ||
| // TODO: why does this make the next test | ||
| // throw an "The operation was aborted." on firefox | ||
| if (!videojs.browser.IS_FIREFOX) { |
There was a problem hiding this comment.
I wonder if this should instead use QUnit.skip? Maybe QUnit[videojs.browser.IS_FIREFOX ? 'skip' : testFn]?
| function(assert) { | ||
| const done = assert.async(); | ||
| let appendsFinished = 0; | ||
| if (!videojs.browser.IS_EDGE) { |
There was a problem hiding this comment.
Same comment about skipping here.
Description
74033->72945We inline 4-5
@babel/runtimehelpers, 4 differentStreamclasses,3base 64 to uin8array functions, and two resolve urls (which each includes their own version of url-toolkit.). This full request removes all the duplicate code and moves it to@videojs/vhs-utils. It also updatesvideojs-generate-rollup-configto deal with sharing@babel/runtimehelpers.Depends on the following generator and small code updates:
Chore/use vhs utils mux.js#295Closes #640
Closes #641