Use faster devtools in webpack (oppia#10228)

* Devtools

* Fixed backend tests

* Fixed backend tests

* Increase backend coverage

* Source maps in lighthouse ci

* Reviews

* reviews

* reviews

* reviews

* reviews

.github/CODEOWNERS

@@ -560,11 +560,7 @@
 /.lighthouserc.json @jimbyo @jameesjohn @vojtechjelinek
 /puppeteer-login-script.js @jimbyo
 /scripts/run_lighthouse_tests.py @jameesjohn @vojtechjelinek
-/webpack.common.config.ts @jameesjohn @vojtechjelinek
-/webpack.common.macros.ts @jameesjohn @vojtechjelinek
-/webpack.dev.config.ts @jameesjohn @vojtechjelinek
-/webpack.prod.config.ts @jameesjohn @vojtechjelinek
-/webpack.terser.config.ts @nishantwrp @vojtechjelinek
+/webpack.*.ts @nishantwrp @vojtechjelinek
 
 
 # Typings.

scripts/build.py

@@ -80,7 +80,10 @@
 PARENT_DIR = os.path.abspath(os.path.join(os.getcwd(), os.pardir))
 UGLIFY_FILE = os.path.join('node_modules', 'uglify-js', 'bin', 'uglifyjs')
 WEBPACK_FILE = os.path.join('node_modules', 'webpack', 'bin', 'webpack.js')
+WEBPACK_DEV_CONFIG = 'webpack.dev.config.ts'
+WEBPACK_DEV_SOURCE_MAPS_CONFIG = 'webpack.dev.soucemap.config.ts'
 WEBPACK_PROD_CONFIG = 'webpack.prod.config.ts'
+WEBPACK_PROD_SOURCE_MAPS_CONFIG = 'webpack.prod.sourcemap.config.ts'
 WEBPACK_TERSER_CONFIG = 'webpack.terser.config.ts'
 
 # Files with these extensions shouldn't be moved to build directory.
@@ -162,6 +165,12 @@
         'meaning that only super admins can access the site.'
     )
 )
+_PARSER.add_argument(
+    '--source_maps',
+    action='store_true',
+    default=False,
+    dest='source_maps',
+    help='Build webpack with source maps.')
 
 
 def generate_app_yaml(deploy_mode=False, maintenance_mode=False):
@@ -1331,7 +1340,13 @@ def main(args=None):
         minify_third_party_libs(THIRD_PARTY_GENERATED_DEV_DIR)
         hashes = generate_hashes()
         if options.deparallelize_terser:
+            if options.source_maps:
+                raise Exception(
+                    'source_maps flag shouldn\'t be used with '
+                    'deparallelize_terser flag.')
             build_using_webpack(WEBPACK_TERSER_CONFIG)
+        elif options.source_maps:
+            build_using_webpack(WEBPACK_PROD_SOURCE_MAPS_CONFIG)
         else:
             build_using_webpack(WEBPACK_PROD_CONFIG)
         generate_app_yaml(

scripts/build_test.py

@@ -991,6 +991,84 @@ def mock_compare_file_count(unused_first_dir, unused_second_dir):
 
         self.assertEqual(check_function_calls, expected_check_function_calls)
 
+    def test_build_with_prod_source_maps(self):
+        check_function_calls = {
+            'build_using_webpack_gets_called': False,
+            'ensure_files_exist_gets_called': False,
+            'modify_constants_gets_called': False,
+            'compare_file_count_gets_called': False
+        }
+        expected_check_function_calls = {
+            'build_using_webpack_gets_called': True,
+            'ensure_files_exist_gets_called': True,
+            'modify_constants_gets_called': True,
+            'compare_file_count_gets_called': True,
+        }
+
+        expected_config_path = build.WEBPACK_PROD_SOURCE_MAPS_CONFIG
+
+        def mock_build_using_webpack(config_path):
+            self.assertEqual(config_path, expected_config_path)
+            check_function_calls['build_using_webpack_gets_called'] = True
+
+        def mock_ensure_files_exist(unused_filepaths):
+            check_function_calls['ensure_files_exist_gets_called'] = True
+
+        def mock_modify_constants(prod_env, maintenance_mode):  # pylint: disable=unused-argument
+            check_function_calls['modify_constants_gets_called'] = True
+
+        def mock_compare_file_count(unused_first_dir, unused_second_dir):
+            check_function_calls['compare_file_count_gets_called'] = True
+
+        ensure_files_exist_swap = self.swap(
+            build, '_ensure_files_exist', mock_ensure_files_exist)
+        build_using_webpack_swap = self.swap(
+            build, 'build_using_webpack', mock_build_using_webpack)
+        modify_constants_swap = self.swap(
+            build, 'modify_constants', mock_modify_constants)
+        compare_file_count_swap = self.swap(
+            build, '_compare_file_count', mock_compare_file_count)
+
+        with ensure_files_exist_swap, build_using_webpack_swap:
+            with modify_constants_swap, compare_file_count_swap:
+                build.main(args=['--prod_env', '--source_maps'])
+
+        self.assertEqual(check_function_calls, expected_check_function_calls)
+
+    def test_cannot_build_with_source_maps_with_terser_config(self):
+        check_function_calls = {
+            'ensure_files_exist_gets_called': False,
+            'modify_constants_gets_called': False
+        }
+        expected_check_function_calls = {
+            'ensure_files_exist_gets_called': True,
+            'modify_constants_gets_called': True
+        }
+
+        def mock_ensure_files_exist(unused_filepaths):
+            check_function_calls['ensure_files_exist_gets_called'] = True
+
+        def mock_modify_constants(prod_env, maintenance_mode):  # pylint: disable=unused-argument
+            check_function_calls['modify_constants_gets_called'] = True
+
+        ensure_files_exist_swap = self.swap(
+            build, '_ensure_files_exist', mock_ensure_files_exist)
+        modify_constants_swap = self.swap(
+            build, 'modify_constants', mock_modify_constants)
+        assert_raises_regexp_context_manager = self.assertRaisesRegexp(
+            Exception,
+            'source_maps flag shouldn\'t be used with '
+            'deparallelize_terser flag.')
+
+        with ensure_files_exist_swap, modify_constants_swap:
+            with assert_raises_regexp_context_manager:
+                build.main(
+                    args=[
+                        '--prod_env', '--source_maps',
+                        '--deparallelize_terser'])
+
+        self.assertEqual(check_function_calls, expected_check_function_calls)
+
     def test_build_with_watcher(self):
         check_function_calls = {
             'ensure_files_exist_gets_called': False,

scripts/release_scripts/deploy.py

@@ -220,7 +220,8 @@ def build_scripts(maintenance_mode):
     # Do a build, while outputting to the terminal.
     python_utils.PRINT('Building and minifying scripts...')
     build_command = [
-        'python', '-m', 'scripts.build', '--prod_env', '--deploy_mode']
+        'python', '-m', 'scripts.build', '--prod_env',
+        '--source_maps', '--deploy_mode']
     if maintenance_mode:
         build_command.append('--maintenance_mode')
     build_process = subprocess.Popen(build_command, stdout=subprocess.PIPE)

scripts/release_scripts/deploy_test.py

@@ -582,7 +582,9 @@ def mock_popen(tokens, stdout):  # pylint: disable=unused-argument
             deploy.build_scripts(False)
         self.assertEqual(
             cmd_tokens,
-            ['python', '-m', 'scripts.build', '--prod_env', '--deploy_mode'])
+            [
+                'python', '-m', 'scripts.build', '--prod_env',
+                '--source_maps', '--deploy_mode'])
 
     def test_build_with_maintenance_mode(self):
         process = subprocess.Popen(['echo', 'test'], stdout=subprocess.PIPE)
@@ -596,7 +598,8 @@ def mock_popen(tokens, stdout):  # pylint: disable=unused-argument
             cmd_tokens,
             [
                 'python', '-m', 'scripts.build',
-                '--prod_env', '--deploy_mode', '--maintenance_mode'
+                '--prod_env', '--source_maps', '--deploy_mode',
+                '--maintenance_mode'
             ]
         )
 
@@ -612,7 +615,9 @@ def mock_popen(tokens, stdout):  # pylint: disable=unused-argument
             deploy.build_scripts(False)
         self.assertEqual(
             cmd_tokens,
-            ['python', '-m', 'scripts.build', '--prod_env', '--deploy_mode'])
+            [
+                'python', '-m', 'scripts.build', '--prod_env',
+                '--source_maps', '--deploy_mode'])
 
     def test_deploy_application(self):
         check_function_calls = {

scripts/run_e2e_tests.py

@@ -147,6 +147,11 @@
     '--contributor_dashboard_enabled', action='store_true',
     help='Run the test after enabling the contributor dashboard page.')
 
+_PARSER.add_argument(
+    '--source_maps',
+    help='Build webpack with source maps.',
+    action='store_true')
+
 # This list contains the sub process triggered by this script. This includes
 # the oppia web server.
 SUBPROCESSES = []
@@ -204,15 +209,18 @@ def is_oppia_server_already_running():
     return running
 
 
-def run_webpack_compilation():
+def run_webpack_compilation(source_maps=False):
     """Runs webpack compilation."""
     max_tries = 5
     webpack_bundles_dir_name = 'webpack_bundles'
     for _ in python_utils.RANGE(max_tries):
         try:
+            webpack_config_file = (
+                build.WEBPACK_DEV_SOURCE_MAPS_CONFIG if source_maps
+                else build.WEBPACK_DEV_CONFIG)
             subprocess.check_call([
                 common.NODE_BIN_PATH, WEBPACK_BIN_PATH, '--config',
-                'webpack.dev.config.ts'])
+                webpack_config_file])
         except subprocess.CalledProcessError as error:
             python_utils.PRINT(error.output)
             sys.exit(error.returncode)
@@ -262,24 +270,31 @@ def setup_and_install_dependencies(skip_install):
         install_chrome_on_travis.main(args=[])
 
 
-def build_js_files(dev_mode_setting, deparallelize_terser=False):
+def build_js_files(
+        dev_mode_setting, deparallelize_terser=False, source_maps=False):
     """Build the javascript files.
 
     Args:
         dev_mode_setting: bool. Represents whether to run the related commands
             in dev mode.
         deparallelize_terser: bool. Represents whether to use webpack
             compilation config that disables parallelism on terser plugin.
+        source_maps: bool. Represents whether to use source maps while
+            building webpack.
     """
     if not dev_mode_setting:
         python_utils.PRINT('  Generating files for production mode...')
+        build_args = ['--prod_env']
+
         if deparallelize_terser:
-            build.main(args=['--prod_env', '--deparallelize_terser'])
-        else:
-            build.main(args=['--prod_env'])
+            build_args.append('--deparallelize_terser')
+        if source_maps:
+            build_args.append('--source_maps')
+
+        build.main(args=build_args)
     else:
         build.main(args=[])
-        run_webpack_compilation()
+        run_webpack_compilation(source_maps=source_maps)
 
 
 @contextlib.contextmanager
@@ -543,7 +558,8 @@ def main(args=None):
         build.modify_constants(prod_env=parsed_args.prod_env)
     else:
         build_js_files(
-            dev_mode, deparallelize_terser=parsed_args.deparallelize_terser)
+            dev_mode, deparallelize_terser=parsed_args.deparallelize_terser,
+            source_maps=parsed_args.source_maps)
     version = parsed_args.chrome_driver_version or get_chrome_driver_version()
     python_utils.PRINT('\n\nCHROMEDRIVER VERSION: %s\n\n' % version)
     start_webdriver_manager(version)

scripts/run_e2e_tests_test.py

@@ -554,6 +554,43 @@ def test_build_js_files_in_prod_mode_with_deparallelize_terser(self):
                     run_e2e_tests.build_js_files(
                         False, deparallelize_terser=True)
 
+    def test_build_js_files_in_prod_mode_with_source_maps(self):
+        run_cmd_swap = self.swap_with_checks(
+            common, 'run_cmd', self.mock_run_cmd, called=False)
+
+        build_main_swap = self.swap_with_checks(
+            build, 'main', self.mock_build_main,
+            expected_kwargs=[{'args': [
+                '--prod_env', '--source_maps']}])
+
+        with self.constant_file_path_swap:
+            with self.node_bin_path_swap, self.webpack_bin_path_swap:
+                with build_main_swap, run_cmd_swap:
+                    run_e2e_tests.build_js_files(
+                        False, source_maps=True)
+
+    def test_webpack_compilation_in_dev_mode_with_source_maps(self):
+        run_cmd_swap = self.swap_with_checks(
+            common, 'run_cmd', self.mock_run_cmd, called=False)
+
+        build_main_swap = self.swap_with_checks(
+            build, 'main', self.mock_build_main,
+            expected_kwargs=[{'args': []}])
+
+        def mock_run_webpack_compilation(source_maps=False):
+            self.assertEqual(source_maps, True)
+
+        run_webpack_compilation_swap = self.swap(
+            run_e2e_tests, 'run_webpack_compilation',
+            mock_run_webpack_compilation)
+
+        with self.constant_file_path_swap:
+            with self.node_bin_path_swap, self.webpack_bin_path_swap:
+                with build_main_swap, run_cmd_swap:
+                    with run_webpack_compilation_swap:
+                        run_e2e_tests.build_js_files(
+                            True, source_maps=True)
+
     def test_tweak_webdriver_manager_on_x64_machine(self):
 
         def mock_is_windows():
@@ -802,7 +839,8 @@ def mock_register(unused_func, unused_arg=None):
         def mock_cleanup():
             return
 
-        def mock_build_js_files(unused_arg, deparallelize_terser=False): # pylint: disable=unused-argument
+        def mock_build_js_files(
+                unused_arg, deparallelize_terser=False, source_maps=False): # pylint: disable=unused-argument
             return
 
         def mock_start_webdriver_manager(unused_arg):
@@ -1083,7 +1121,8 @@ def mock_register(unused_func, unused_arg=None):
         def mock_cleanup():
             return
 
-        def mock_build_js_files(unused_arg, deparallelize_terser=False): # pylint: disable=unused-argument
+        def mock_build_js_files(
+                unused_arg, deparallelize_terser=False, source_maps=False): # pylint: disable=unused-argument
             return
 
         def mock_start_webdriver_manager(unused_arg):
@@ -1201,7 +1240,8 @@ def mock_register(unused_func, unused_arg=None):
         def mock_cleanup():
             return
 
-        def mock_build_js_files(unused_arg, deparallelize_terser=False): # pylint: disable=unused-argument
+        def mock_build_js_files(
+                unused_arg, deparallelize_terser=False, source_maps=False): # pylint: disable=unused-argument
             return
 
         def mock_start_webdriver_manager(unused_arg):

scripts/run_lighthouse_tests.py

@@ -105,7 +105,10 @@ def main():
     atexit.register(cleanup)
 
     python_utils.PRINT('Building files in production mode.')
-    build.main(args=['--prod_env'])
+    # We are using --source_maps here, so that we have at least one CI check
+    # that builds using source maps in prod env. This is to ensure that
+    # there are no issues while deploying oppia.
+    build.main(args=['--prod_env', '--source_maps'])
     build.modify_constants(prod_env=True)
     start_google_app_engine_server()
     common.wait_for_port_to_be_open(GOOGLE_APP_ENGINE_PORT)

scripts/start.py

@@ -78,6 +78,11 @@
         'optional; if specified, does not automatically restart when files are '
         'changed.'),
     action='store_true')
+_PARSER.add_argument(
+    '--source_maps',
+    help=(
+        'optional; if specified, build webpack with source maps.'),
+    action='store_true')
 
 PORT_NUMBER_FOR_GAE_SERVER = 8181
 
@@ -122,6 +127,8 @@ def main(args=None):
     build_args = ['--prod_env'] if parsed_args.prod_env else []
     if parsed_args.maintenance_mode:
         build_args.append('--maintenance_mode')
+    if parsed_args.source_maps:
+        build_args.append('--source_maps')
     build.main(args=build_args)
     app_yaml_filepath = 'app.yaml' if parsed_args.prod_env else 'app_dev.yaml'
 
@@ -134,11 +141,15 @@ def main(args=None):
     if not parsed_args.prod_env:
         # In prod mode webpack is launched through scripts/build.py
         python_utils.PRINT('Compiling webpack...')
+        webpack_config_file = (
+            build.WEBPACK_DEV_SOURCE_MAPS_CONFIG if parsed_args.source_maps
+            else build.WEBPACK_DEV_CONFIG)
         background_processes.append(subprocess.Popen([
             common.NODE_BIN_PATH,
             os.path.join(
                 common.NODE_MODULES_PATH, 'webpack', 'bin', 'webpack.js'),
-            '--config', 'webpack.dev.config.ts', '--watch']))
+            '--config', webpack_config_file, '--watch']))
+
         # Give webpack few seconds to do the initial compilation.
         time.sleep(10)
 

webpack.dev.config.ts

@@ -26,7 +26,7 @@ module.exports = merge(common, {
     filename: '[name].bundle.js',
     path: path.resolve(__dirname, 'webpack_bundles')
   },
-  devtool: 'inline-source-map',
+  devtool: 'eval',
   watchOptions: {
     aggregateTimeout: 500,
     poll: 1000