fix: remove global and use 'videojs-global-compat' #1595

ronalduQualabs · 2025-11-05T14:42:50Z

Description

remove global and use 'videojs-global-compat' to fix videojs/video.js#9103

If it's a feature or enhancement please be as detailed as possible.
If it's a bug fix, please link the issue that it fixes or describe the bug in as much detail.

Specific Changes proposed

Please list the specific changes involved in this pull request.

Requirements Checklist

Feature implemented / Bug fixed
If necessary, more likely in a feature request than a bug fix
- Unit Tests updated or fixed
- Docs/guides updated
- Example created (starter template on JSBin)
Reviewed by Two Core Contributors

welcome · 2025-11-05T14:42:54Z

💖 Thanks for opening this pull request! 💖

Things that will help get your PR across the finish line:

Run npm run lint -- --errors locally to catch formatting errors earlier.
Include tests when adding/changing behavior.
Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

mister-ben

The videos-global-compat probably ought to be in the videojs GitHub and NPM orgs.
The lockfile version changes from v1 to v3. Not sure if that might add some unexpected changes or overheads to future maintenance.
The old global package wasn't included in the builds because they're set up as externals in the rollup configuration. With the new package included, dist/videojs-http-streaming.js is 5.7% larger, min is 7%. That would need to change in the shared rollup configuration in videojs-generate-rollup-config:
- https://github.com/videojs/videojs-generate-rollup-config/blob/b9f3943f0301cdf8a191825308e963135fcee180/index.js#L82
- https://github.com/videojs/videojs-generate-rollup-config/blob/b9f3943f0301cdf8a191825308e963135fcee180/index.js#L139-L141
There are some replacements of "global" in test files that should be left. Probably not breaking, but confusing.

mister-ben · 2025-11-05T15:11:02Z

package.json

    "mux.js": "7.1.0",
-    "video.js": "^7 || ^8"
+    "video.js": "^7 || ^8",
+    "videojs-global-compat": "^1.0.1"


The repo and package probably ought to be in the video.js GitHub and NPM orgs.

test/xhr.test.js

mister-ben · 2025-11-06T17:59:24Z

test/videojs-http-streaming.test.js

  assert.equal(beforeLocalRequestCalled, 2, 'local beforeRequest was called twice ' +
                                           'for the media playlist and media');
-  assert.equal(beforeGlobalRequestCalled, 1, 'global beforeRequest was called once ' +
+  assert.equal(beforeGlobalRequestCalled, 1, 'videojs-global-compat beforeRequest was called once ' +


Suggested change

assert.equal(beforeGlobalRequestCalled, 1, 'videojs-global-compat beforeRequest was called once ' +

assert.equal(beforeGlobalRequestCalled, 1, 'global beforeRequest was called once ' +

mister-ben · 2025-11-06T17:59:36Z

test/videojs-http-streaming.test.js


  assert.equal(onRequestHookCallCountGlobal, 0, 'no onRequest global hooks called');
-  assert.equal(actualRequestUrlGlobal, undefined, 'global request url undefined');
+  assert.equal(actualRequestUrlGlobal, undefined, 'videojs-global-compat request url undefined');


Suggested change

assert.equal(actualRequestUrlGlobal, undefined, 'videojs-global-compat request url undefined');

assert.equal(actualRequestUrlGlobal, undefined, 'globalrequest url undefined');

mister-ben · 2025-11-06T17:59:51Z

test/xhr.test.js


  this.xhr(defaultOptions);
-  assert.equal(this.requests.shift().url, 'global', 'url changed with global override');
+  assert.equal(this.requests.shift().url, 'videojs-global-compat', 'url changed with global override');


Suggested change

assert.equal(this.requests.shift().url, 'videojs-global-compat', 'url changed with global override');

assert.equal(this.requests.shift().url, 'global', 'url changed with global override');

mister-ben · 2025-11-06T18:00:08Z

test/xhr.test.js


  videojs.Vhs.xhr.beforeRequest = () => {
-    return { uri: 'global-newOptions'};
+    return { uri: 'videojs-global-compat-newOptions'};


Suggested change

return { uri: 'videojs-global-compat-newOptions'};

return { uri: 'global-newOptions'};

mister-ben · 2025-11-06T18:00:24Z

test/xhr.test.js


  this.xhr(defaultOptions);
-  assert.equal(this.requests.shift().url, 'global-newOptions', 'url changed with global override');
+  assert.equal(this.requests.shift().url, 'videojs-global-compat-newOptions', 'url changed with global override');


Suggested change

assert.equal(this.requests.shift().url, 'videojs-global-compat-newOptions', 'url changed with global override');

assert.equal(this.requests.shift().url, 'global-newOptions', 'url changed with global override');

mister-ben · 2025-11-06T18:00:35Z

test/xhr.test.js

  xhrRequest = this.requests.shift();

-  assert.equal(xhrRequest.url, 'global', 'url changed with global onRequest hooks');
+  assert.equal(xhrRequest.url, 'videojs-global-compat', 'url changed with global onRequest hooks');


Suggested change

assert.equal(xhrRequest.url, 'videojs-global-compat', 'url changed with global onRequest hooks');

assert.equal(xhrRequest.url, 'global', 'url changed with global onRequest hooks');

mister-ben · 2025-11-06T18:00:48Z

test/xhr.test.js

  this.xhr(defaultOptions);
  xhrRequest = this.requests.shift();
-  assert.equal(xhrRequest.url, 'global', 'url changed with player onRequest hooks');
+  assert.equal(xhrRequest.url, 'videojs-global-compat', 'url changed with player onRequest hooks');


Suggested change

assert.equal(xhrRequest.url, 'videojs-global-compat', 'url changed with player onRequest hooks');

assert.equal(xhrRequest.url, 'global', 'url changed with player onRequest hooks');

mister-ben · 2025-11-06T18:01:01Z

test/xhr.test.js

    assert.equal(response.headers.foo, 'bar', 'expected headers');
    assert.equal(response.statusCode, 200, 'expected statusCode');
-    assert.equal(globalHookCallCount, 0, 'global response hooks not called yet');
+    assert.equal(globalHookCallCount, 0, 'videojs-global-compat response hooks not called yet');


Suggested change

assert.equal(globalHookCallCount, 0, 'videojs-global-compat response hooks not called yet');

assert.equal(globalHookCallCount, 0, 'globalresponse hooks not called yet');

codecov · 2025-11-06T18:41:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.02%. Comparing base (bd810ea) to head (6cee06f).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1595      +/-   ##
==========================================
+ Coverage   84.00%   84.02%   +0.01%     
==========================================
  Files          44       44              
  Lines       11710    11710              
  Branches     2625     2625              
==========================================
+ Hits         9837     9839       +2     
+ Misses       1873     1871       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ronalduQualabs added 2 commits November 5, 2025 11:39

fix: remove global and use 'videojs-global-compat'

9af9a9f

restore video.js 7 in deps

6dbd193

mister-ben reviewed Nov 5, 2025

View reviewed changes

pr fix

a673e7b

mister-ben reviewed Nov 6, 2025

View reviewed changes

update package name

bf609b9

restore lockfileVersion

6cee06f

	assert.equal(beforeGlobalRequestCalled, 1, 'videojs-global-compat beforeRequest was called once ' +
	assert.equal(beforeGlobalRequestCalled, 1, 'global beforeRequest was called once ' +

	assert.equal(actualRequestUrlGlobal, undefined, 'videojs-global-compat request url undefined');
	assert.equal(actualRequestUrlGlobal, undefined, 'globalrequest url undefined');

	assert.equal(this.requests.shift().url, 'videojs-global-compat', 'url changed with global override');
	assert.equal(this.requests.shift().url, 'global', 'url changed with global override');

	return { uri: 'videojs-global-compat-newOptions'};
	return { uri: 'global-newOptions'};

	assert.equal(xhrRequest.url, 'videojs-global-compat', 'url changed with global onRequest hooks');
	assert.equal(xhrRequest.url, 'global', 'url changed with global onRequest hooks');

	assert.equal(globalHookCallCount, 0, 'videojs-global-compat response hooks not called yet');
	assert.equal(globalHookCallCount, 0, 'globalresponse hooks not called yet');

fix: remove global and use 'videojs-global-compat' #1595

Are you sure you want to change the base?

fix: remove global and use 'videojs-global-compat' #1595

Uh oh!

Conversation

ronalduQualabs commented Nov 5, 2025

Description

Specific Changes proposed

Requirements Checklist

Uh oh!

welcome bot commented Nov 5, 2025

Uh oh!

mister-ben left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

codecov bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Nov 6, 2025 •

edited

Loading