Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(FEC-13175): Upgrade Webpack deps and fix source-maps #211

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

JonathanTGold
Copy link
Contributor

@JonathanTGold JonathanTGold commented May 22, 2023

Description of the Changes

  • Upgrade the Webpack deps to align with the latest version
  • Refactor the webpack config to the newest syntax and best practices to reduce bundle
  • Fix the source-map configuration to enable proper debugging
  • remove deprecated/unnecessary dependencies
  • remove redundant/unused scripts from the package.json
  • remove dist file from the git tracked files on git
  • add Safari & Firefox to script test (currently only chrome is covered)

The updated Babel configuration for the webpage aims to remove support for Internet Explorer (IE), reduce the bundle size, and improve code quality. The changes can be described as follows:

  • The targets: 'defaults' flag sets the target browsers to the default configuration, which includes modern browsers but excludes IE.
  • In addition, the loose flag has been removed from the configuration, resulting in its default value of false. The loose flag, when set to true, allows Babel to apply less strict transformations, potentially resulting in smaller and faster code. However, by setting it to false, you ensure more accurate and safer transformations that adhere strictly to the ECMAScript specifications.

The benefits of this updated configuration are:

  1. Improved code reliability: By setting loose to false by default, you ensure that Babel applies transformations that strictly adhere to the ECMAScript specifications. This results in more reliable code that is less prone to subtle bugs and compatibility issues.
  2. Smaller bundle size: The removal of IE support and the default loose: false setting allows Babel to perform more accurate transformations, which can eliminate unnecessary code and reduce the overall bundle size. This results in faster loading times for your webpage, especially for modern browsers.
  3. Enhanced code quality: By adhering to the ECMAScript specifications and focusing on modern browser support, the updated configuration encourages the use of cleaner and more standard-compliant code. This can lead to improved code quality, easier maintenance, and better collaboration among developers.

By updating the Babel configuration in this way, you not only remove IE support and reduce bundle size but also ensure code reliability and enhance code quality by adhering to strict ECMAScript standards. This helps optimize the performance of your webpage while promoting maintainable and reliable code.

Bundle size effects:

Before:
playkit-ott-provider.js 31.3 KiB
playkit-ovp-provider.js 47.7 KiB
playkit-analytics-service.js 10.5 KiB
playkit-bookmark-service.js 9.17 KiB
playkit-stats-service.js 9.82 KiB

After:
asset playkit-ovp-provider.js 35.7 KiB
asset playkit-ott-provider.js 22.2 KiB
asset playkit-analytics-service.js 7.81 KiB
asset playkit-stats-service.js 7.16 KiB
asset playkit-bookmark-service.js 6.76 KiB

solves FEC-13175

CheckLists

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • test are passing in local environment
  • Travis tests are passing (or test results are not worse than on master branch :))
  • Docs have been updated

}
};
const webpackConfig = require('./webpack.config')('development', {mode: 'development'});
delete webpackConfig.entry;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why ?

@@ -0,0 +1,5 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do you need a separate package.json for each ?

const webpack = require('webpack');
const path = require('path');
const packageData = require('./package.json');
const TerserPlugin = require('terser-webpack-plugin');
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does terser provide ?

logLevel: config.LOG_INFO,
autoWatch: false,
browsers: ['ChromeHeadless'],
singleRun: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

does test:watch still work when singleRun is true in the config file ?

"build": "webpack --mode production",
"dev": "webpack-dev-server --mode development",
"watch": "webpack --progress --colors --watch --mode development",
"build:dev": "webpack serve --open --mode=development",
Copy link
Collaborator

@SivanA-Kaltura SivanA-Kaltura May 23, 2023

Choose a reason for hiding this comment

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

doesn't the devops script count on a "build" task existing ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants