Skip to content

Commit

Permalink
chore: Remove safe-json-parse (#8790)
Browse files Browse the repository at this point in the history
## Description
Removes an old unmaintained dependency that isn't needed any more.

## Specific Changes proposed
Replace safe-json-parse with `JSON.parse`

## Requirements Checklist
- [x] Feature implemented / Bug fixed
- [ ] If necessary, more likely in a feature request than a bug fix
- [x] Change has been verified in an actual browser (Chrome, Firefox,
IE)
  - [ ] Unit Tests updated or fixed
  - [ ] Docs/guides updated
- [ ] Example created ([starter template on
JSBin](https://codepen.io/gkatsev/pen/GwZegv?editors=1000#0))
- [x] Has no DOM changes which impact accessiblilty or trigger warnings
(e.g. Chrome issues tab)
  - [x] Has no changes to JSDoc which cause `npm run docs:api` to error
- [ ] Reviewed by Two Core Contributors
  • Loading branch information
mister-ben authored Aug 11, 2024
1 parent 57c27f8 commit 3380d33
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 10 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@
"m3u8-parser": "^7.1.0",
"mpd-parser": "^1.2.2",
"mux.js": "^7.0.1",
"safe-json-parse": "4.0.0",
"videojs-contrib-quality-levels": "4.1.0",
"videojs-font": "4.2.0",
"videojs-vtt.js": "0.15.5"
Expand Down
12 changes: 5 additions & 7 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { bufferedPercent } from './utils/buffer.js';
import * as stylesheet from './utils/stylesheet.js';
import FullscreenApi from './fullscreen-api.js';
import MediaError from './media-error.js';
import safeParseTuple from 'safe-json-parse/tuple';
import {merge} from './utils/obj';
import {silencePromise, isPromise} from './utils/promise';
import textTrackConverter from './tracks/text-track-list-converter.js';
Expand Down Expand Up @@ -5231,13 +5230,12 @@ class Player extends Component {
// Check if data-setup attr exists.
if (dataSetup !== null) {
// Parse options JSON
// If empty string, make it a parsable json object.
const [err, data] = safeParseTuple(dataSetup || '{}');

if (err) {
log.error(err);
try {
// If empty string, make it a parsable json object.
Object.assign(tagOptions, JSON.parse(dataSetup || '{}'));
} catch (e) {
log.error('data-setup', e);
}
Object.assign(tagOptions, data);
}

Object.assign(baseOptions, tagOptions);
Expand Down
19 changes: 19 additions & 0 deletions test/unit/setup.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* eslint-env qunit */
import TestHelpers from './test-helpers.js';
import sinon from 'sinon';
import window from 'global/window';

QUnit.module('Setup');

Expand All @@ -19,3 +21,20 @@ QUnit.test('should set options from data-setup even if autoSetup is not called b
assert.ok(player.options_.playsinline === true);
player.dispose();
});

QUnit.test('should log an error if data-setup has invalid JSON', function(assert) {
const logError = sinon.spy(window.console, 'error');

const el = TestHelpers.makeTag();

el.setAttribute(
'data-setup',
"{'controls': true}"
);

const player = TestHelpers.makePlayer({}, el);

assert.ok(logError.calledWith('VIDEOJS:', 'ERROR:', 'data-setup'));
player.dispose();
window.console.error.restore();
});
3 changes: 1 addition & 2 deletions test/unit/tracks/text-track-settings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import TextTrackSettings from '../../../src/js/tracks/text-track-settings.js';
import TestHelpers from '../test-helpers.js';
import * as Events from '../../../src/js/utils/events.js';
import safeParseTuple from 'safe-json-parse/tuple';
import sinon from 'sinon';
import window from 'global/window';
import Component from '../../../src/js/component.js';
Expand Down Expand Up @@ -120,7 +119,7 @@ QUnit.test('should update settings', function(assert) {
Events.trigger(player.$('.vjs-done-button'), 'click');

assert.deepEqual(
safeParseTuple(window.localStorage.getItem('vjs-text-track-settings'))[1],
JSON.parse(window.localStorage.getItem('vjs-text-track-settings')),
newSettings,
'values are saved'
);
Expand Down

0 comments on commit 3380d33

Please # to comment.