From 820ef382bc69c183c7bdd80536c981a7ffd6bdb0 Mon Sep 17 00:00:00 2001 From: mister-ben <1676039+mister-ben@users.noreply.github.com> Date: Mon, 12 Aug 2024 21:47:47 +0200 Subject: [PATCH] fix(types): Add has|usingPlugin to typedef by adding stubs which are removed from builds (#8811) ## Description tsc doesn't understand mixins and ignores jsdoc not followed by code. The jsdoc for the plugin methods `usingPlugin()` and `hasPlugin()` in Player are being ignored. To get them included in type outputs we need to have otherwise unnecessary stubs codes, as we already have for `on()` etc, which adds unnecessary, even if a small amount of, code to the outputs. ## Specific Changes proposed * Slight refactor of Player to include those stubs. * Adds a rollup plugin to delete lines between certain comments, so those stubs are deleted from the outputs. * Applies those comments to the `on()` etc stubs in Component and the new plugin stubs in Player. Any code surrounded by these comments, and the comments themselves, is deleted from the dist and test builds: ```js /* start-delete-from-build */ console.log('hi'); /* start-delete-from-build */ ``` Compared to main, video.min.js is 53 bytes smaller. ## Requirements Checklist - [ ] Feature implemented / Bug fixed - [ ] If necessary, more likely in a feature request than a bug fix - [ ] 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)) - [ ] Has no DOM changes which impact accessiblilty or trigger warnings (e.g. Chrome issues tab) - [ ] Has no changes to JSDoc which cause `npm run docs:api` to error - [ ] Reviewed by Two Core Contributors --- build/rollup-exclude-lines.js | 41 ++++++++++++++++++++++ package.json | 1 + rollup.config.js | 25 +++++++++++++ src/js/component.js | 10 ++++++ src/js/player.js | 66 ++++++++++++++++++++--------------- 5 files changed, 115 insertions(+), 28 deletions(-) create mode 100644 build/rollup-exclude-lines.js diff --git a/build/rollup-exclude-lines.js b/build/rollup-exclude-lines.js new file mode 100644 index 0000000000..a9304f76a5 --- /dev/null +++ b/build/rollup-exclude-lines.js @@ -0,0 +1,41 @@ +/** + * Remove parts of files from outputs. Everything between a pair of `/* start-delete-from-build *\u002f` + * and `/* end-delete-from-build *\u002f` comments + * + * Based on https://github.com/se-panfilov/rollup-plugin-strip-code + */ + +import { createFilter } from '@rollup/pluginutils'; + +const START_COMMENT = 'start-delete-from-build'; +const END_COMMENT = 'end-delete-from-build'; + +/** + * Remove lines of code surrounded by comments + * + * @param {Object} [options] Options + * @param {string} [options.include] Files to inlcude + * @param {string} [options.exclude] Files to exclude + * @param {string} [options.startComment] Starting keywork, default start-delete-from-build + * @param {string} [options.endComment] Eding keywork, default end-delete-from-build + * @param {RegExp} [options.pattern] Custom regex + * @return void + */ +export default function excludeLines(options = {}) { + // assume that the myPlugin accepts options of `options.include` and `options.exclude` + const filter = createFilter(options.include, options.exclude); + + return { + transform(code, id) { + if (!filter(id)) { + return; + } + + const startComment = options.startComment || START_COMMENT; + const endComment = options.endComment || END_COMMENT; + const defaultPattern = new RegExp(`([\\t ]*\\/\\* ?${startComment} ?\\*\\/)[\\s\\S]*?(\\/\\* ?${endComment} ?\\*\\/[\\t ]*\\n?)`, 'g'); + + return code.replace(options.pattern || defaultPattern, ''); + } + }; +} diff --git a/package.json b/package.json index a221665d08..007d85ae81 100644 --- a/package.json +++ b/package.json @@ -104,6 +104,7 @@ "@babel/preset-env": "^7.9.0", "@rollup/plugin-image": "^3.0.2", "@rollup/plugin-replace": "^2.4.1", + "@rollup/pluginutils": "^5.1.0", "@types/node": "^18.8.3", "access-sniff": "^3.2.0", "autoprefixer": "^10.2.5", diff --git a/rollup.config.js b/rollup.config.js index 4a97d0156e..c24f5a9713 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -17,6 +17,7 @@ import image from '@rollup/plugin-image'; import istanbul from 'rollup-plugin-istanbul'; import externalGlobals from 'rollup-plugin-external-globals'; import svg from 'rollup-plugin-svg'; +import excludeLines from './build/rollup-exclude-lines'; const excludeCoverage = [ 'test/**', @@ -143,6 +144,9 @@ export default cliargs => [ }, external: externals.browser, plugins: [ + excludeLines({ + include: 'src/js/**' + }), alias({ 'video.js': path.resolve(__dirname, './src/js/video.js') }), @@ -169,6 +173,9 @@ export default cliargs => [ }, external: externals.browser, plugins: [ + excludeLines({ + include: 'src/js/**' + }), alias({ 'video.js': path.resolve(__dirname, './src/js/video.js') }), @@ -193,6 +200,9 @@ export default cliargs => [ }, external: externals.test, plugins: [ + excludeLines({ + include: 'src/js/**' + }), multiEntry({exports: false}), alias({ 'video.js': path.resolve(__dirname, './src/js/video.js') @@ -228,6 +238,9 @@ export default cliargs => [ ], external: externals.module, plugins: [ + excludeLines({ + include: 'src/js/**' + }), alias({ 'video.js': path.resolve(__dirname, './src/js/video.js'), 'videojs-contrib-quality-levels': path.resolve(__dirname, './node_modules/videojs-contrib-quality-levels/dist/videojs-contrib-quality-levels.es.js'), @@ -260,6 +273,9 @@ export default cliargs => [ external: externals.browser, plugins: [ primedIgnore, + excludeLines({ + include: 'src/js/**' + }), alias({ 'video.js': path.resolve(__dirname, './src/js/video.js') }), @@ -292,6 +308,9 @@ export default cliargs => [ ], external: externals.module, plugins: [ + excludeLines({ + include: 'src/js/**' + }), json(), primedBabel, svg(), @@ -313,6 +332,9 @@ export default cliargs => [ external: externals.browser, plugins: [ primedResolve, + excludeLines({ + include: 'src/js/**' + }), json(), primedExternalGlobals, primedCjs, @@ -337,6 +359,9 @@ export default cliargs => [ plugins: [ primedIgnore, primedResolve, + excludeLines({ + include: 'src/js/**' + }), json(), primedExternalGlobals, primedCjs, diff --git a/src/js/component.js b/src/js/component.js index ad8e67f627..b64e19c416 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -151,7 +151,9 @@ class Component { * @param {Function} fn * The function to call with `EventTarget`s */ + /* start-delete-from-build */ on(type, fn) {} + /* end-delete-from-build */ /** * Removes an `event listener` for a specific event from an instance of `EventTarget`. @@ -164,7 +166,9 @@ class Component { * @param {Function} [fn] * The function to remove. If not specified, all listeners managed by Video.js will be removed. */ + /* start-delete-from-build */ off(type, fn) {} + /* end-delete-from-build */ /** * This function will add an `event listener` that gets triggered only once. After the @@ -177,7 +181,9 @@ class Component { * @param {Function} fn * The function to be called once for each event name. */ + /* start-delete-from-build */ one(type, fn) {} + /* end-delete-from-build */ /** * This function will add an `event listener` that gets triggered only once and is @@ -191,7 +197,9 @@ class Component { * @param {Function} fn * The function to be called once for each event name. */ + /* start-delete-from-build */ any(type, fn) {} + /* end-delete-from-build */ /** * This function causes an event to happen. This will then cause any `event listeners` @@ -212,7 +220,9 @@ class Component { * @param {Object} [hash] * Optionally extra argument to pass through to an event listener */ + /* start-delete-from-build */ trigger(event, hash) {} + /* end-delete-from-build */ /** * Dispose of the `Component` and all child components. diff --git a/src/js/player.js b/src/js/player.js index 8282f9c409..ddffa5bdca 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -5323,6 +5323,44 @@ class Player extends Component { */ this.trigger('playbackrateschange'); } + + /** + * Reports whether or not a player has a plugin available. + * + * This does not report whether or not the plugin has ever been initialized + * on this player. For that, [usingPlugin]{@link Player#usingPlugin}. + * + * @method hasPlugin + * @param {string} name + * The name of a plugin. + * + * @return {boolean} + * Whether or not this player has the requested plugin available. + */ + /* start-delete-from-build */ + hasPlugin(name) { + return false; + } + /* end-delete-from-build */ + + /** + * Reports whether or not a player is using a plugin by name. + * + * For basic plugins, this only reports whether the plugin has _ever_ been + * initialized on this player. + * + * @method Player#usingPlugin + * @param {string} name + * The name of a plugin. + * + * @return {boolean} + * Whether or not this player is using the requested plugin. + */ + /* start-delete-from-build */ + usingPlugin(name) { + return false; + } + /* end-delete-from-build */ } /** @@ -5525,33 +5563,5 @@ TECH_EVENTS_RETRIGGER.forEach(function(event) { * @type {Event} */ -/** - * Reports whether or not a player has a plugin available. - * - * This does not report whether or not the plugin has ever been initialized - * on this player. For that, [usingPlugin]{@link Player#usingPlugin}. - * - * @method Player#hasPlugin - * @param {string} name - * The name of a plugin. - * - * @return {boolean} - * Whether or not this player has the requested plugin available. - */ - -/** - * Reports whether or not a player is using a plugin by name. - * - * For basic plugins, this only reports whether the plugin has _ever_ been - * initialized on this player. - * - * @method Player#usingPlugin - * @param {string} name - * The name of a plugin. - * - * @return {boolean} - * Whether or not this player is using the requested plugin. - */ - Component.registerComponent('Player', Player); export default Player;