Fix part of oppia#6732: Migrate RTCs to webpack (oppia#6882)

* migrate RTCs to webpack * add overview * modify tests webpack config * add requires to RTCs * remove html script-import file * modify webpack configs * remove file from CODEOWNERS * Change test_html_contains_all_imports so that it looks in the ts file * improve the backend test
saketringsia · Jun 10, 2019 · c2b1fa3 · c2b1fa3
1 parent d7f23ee
commit c2b1fa3
Show file tree

Hide file tree

Showing 23 changed files with 96 additions and 40 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -351,7 +351,6 @@
 /core/templates/dev/head/mathjaxConfig.ts @AllanYangZhou @bansalnitish
 /extensions/rich_text_components/ @AllanYangZhou @bansalnitish
 /core/templates/dev/head/components/ck-editor-helpers/ck-editor-widgets.initializer.ts @AllanYangZhou @bansalnitish
-/core/templates/dev/head/components/ck-editor-helpers/rich-text-components.template.html @AllanYangZhou @bansalnitish
 /core/templates/dev/head/services/AutoplayedVideosService*.ts @AllanYangZhou @bansalnitish
 /core/templates/dev/head/services/RteHelperService*.ts @AllanYangZhou @bansalnitish
 /extensions/ckeditor_plugins/ @AllanYangZhou @bansalnitish

diff --git a/core/domain/rte_component_registry_test.py b/core/domain/rte_component_registry_test.py
@@ -179,36 +179,28 @@ def test_rte_components_are_valid(self):
             self._validate_customization_arg_specs(
                 component_specs['customization_arg_specs'])  # pylint: disable=protected-access
 
-    def test_html_contains_all_imports(self):
+    def test_require_file_contains_all_imports(self):
         """Test that the rich_text_components.html file contains script-imports
         for all directives of all RTE components.
         """
 
-        ts_files_paths = []
+        rtc_ts_filenames = []
         for component_id in feconf.ALLOWED_RTE_EXTENSIONS:
             component_dir = os.path.join(
                 feconf.RTE_EXTENSIONS_DIR, component_id)
             directives_dir = os.path.join(component_dir, 'directives')
             directive_filenames = os.listdir(directives_dir)
-            ts_files_paths.extend(
-                os.path.join(directives_dir, filename) for filename
+            rtc_ts_filenames.extend(
+                filename for filename
                 in directive_filenames if filename.endswith('.ts'))
 
-        ts_files_paths.sort()
-        js_files_paths = [path.replace('.ts', '.js') for path in ts_files_paths]
-        prefix = '<script src="/'
-        suffix = '"></script>'
-        html_script_tags = [
-            '%s%s%s' % (prefix, path, suffix) for path in js_files_paths]
-        generated_html = '\n'.join(html_script_tags)
-
-        rtc_html_file = os.path.join(
-            feconf.FRONTEND_TEMPLATES_DIR, 'components', 'ck-editor-helpers',
-            'rich-text-components.template.html')
-        with open(rtc_html_file, 'r') as f:
-            rtc_html_file_contents = f.read()
-
-        self.assertEqual(generated_html, rtc_html_file_contents.strip())
+        rtc_ts_file = os.path.join(
+            feconf.RTE_EXTENSIONS_DIR, 'richTextComponentsRequires.ts')
+        with open(rtc_ts_file, 'r') as f:
+            rtc_require_file_contents = f.read()
+
+        for rtc_ts_filename in rtc_ts_filenames:
+            self.assertIn(rtc_ts_filename, rtc_require_file_contents)
 
 
 class RteComponentRegistryUnitTests(test_utils.GenericTestBase):

diff --git a/core/templates/dev/head/components/ck-editor-helpers/ck-editor-widgets.initializer.ts b/core/templates/dev/head/components/ck-editor-helpers/ck-editor-widgets.initializer.ts
@@ -17,6 +17,7 @@
  * text components.
  */
 
+require('rich_text_components/richTextComponentsRequires.ts');
 require('services/HtmlEscaperService.ts');
 require('services/RteHelperService.ts');
 

diff --git a/core/templates/dev/head/components/ck-editor-helpers/rich-text-components.template.html b/core/templates/dev/head/components/ck-editor-helpers/rich-text-components.template.html
diff --git a/core/templates/dev/head/pages/admin/admin.html b/core/templates/dev/head/pages/admin/admin.html
@@ -45,7 +45,6 @@
         <script src="/dist/<%= htmlWebpackPlugin.files.js[chunk] %>"></script>
       <% } %>
     <% } %>
-    {% include 'components/ck-editor-helpers/rich-text-components.template.html' %}
 
     <script>
       {{ value_generators_js }}

diff --git a/core/templates/dev/head/pages/creator_dashboard/creator_dashboard.html b/core/templates/dev/head/pages/creator_dashboard/creator_dashboard.html
@@ -911,7 +911,6 @@ <h2>Suggestions to review</h2>
       <script src="/dist/<%= htmlWebpackPlugin.files.js[chunk] %>"></script>
     <% } %>
   <% } %>
-  {% include 'components/ck-editor-helpers/rich-text-components.template.html' %}
   {{ interaction_templates }}
   {{ visualizations_html }}
   {{ dependencies_html }}

diff --git a/core/templates/dev/head/pages/exploration_editor/exploration_editor.html b/core/templates/dev/head/pages/exploration_editor/exploration_editor.html
@@ -164,7 +164,6 @@
       <script src="/dist/<%= htmlWebpackPlugin.files.js[chunk] %>"></script>
     <% } %>
   <% } %>
-  {% include 'components/ck-editor-helpers/rich-text-components.template.html' %}
   {{ interaction_templates }}
   {{ visualizations_html }}
   {{ dependencies_html }}

diff --git a/core/templates/dev/head/pages/exploration_player/exploration_player.html b/core/templates/dev/head/pages/exploration_player/exploration_player.html
@@ -82,7 +82,6 @@
       <script src="/dist/<%= htmlWebpackPlugin.files.js[chunk] %>"></script>
     <% } %>
   <% } %>
-  {% include 'components/ck-editor-helpers/rich-text-components.template.html' %}
   {{ interaction_templates }}
   {{ dependencies_html }}
 {% endblock footer_js %}
diff --git a/core/templates/dev/head/pages/practice-session-page/practice-session-page.mainpage.html b/core/templates/dev/head/pages/practice-session-page/practice-session-page.mainpage.html
@@ -78,7 +78,6 @@
       <script src="/dist/<%= htmlWebpackPlugin.files.js[chunk] %>"></script>
     <% } %>
   <% } %>
-  {% include 'components/ck-editor-helpers/rich-text-components.template.html' %}
   {{ interaction_templates }}
   {{ dependencies_html }}
 {% endblock footer_js %}
diff --git a/core/templates/dev/head/pages/skill_editor/skill_editor.html b/core/templates/dev/head/pages/skill_editor/skill_editor.html
@@ -81,7 +81,6 @@
       <script src="/dist/<%= htmlWebpackPlugin.files.js[chunk] %>"></script>
     <% } %>
   <% } %>
-  {% include 'components/ck-editor-helpers/rich-text-components.template.html' %}
   {{ interaction_templates }}
   {{ visualizations_html }}
   {{ dependencies_html }}

diff --git a/core/templates/dev/head/pages/story_editor/story_editor.html b/core/templates/dev/head/pages/story_editor/story_editor.html
@@ -45,5 +45,4 @@
       <script src="/dist/<%= htmlWebpackPlugin.files.js[chunk] %>"></script>
     <% } %>
   <% } %>
-  {% include 'components/ck-editor-helpers/rich-text-components.template.html' %}
 {% endblock footer_js %}
diff --git a/core/templates/dev/head/pages/topic_editor/topic_editor.html b/core/templates/dev/head/pages/topic_editor/topic_editor.html
@@ -81,7 +81,6 @@
       <script src="/dist/<%= htmlWebpackPlugin.files.js[chunk] %>"></script>
     <% } %>
   <% } %>
-  {% include 'components/ck-editor-helpers/rich-text-components.template.html' %}
   {{ interaction_templates }}
   {{ visualizations_html }}
   {{ dependencies_html }}

diff --git a/core/tests/karma.conf.ts b/core/tests/karma.conf.ts
@@ -154,7 +154,10 @@ module.exports = function(config) {
     webpack: {
       mode: 'development',
       resolve: {
-        modules: ['core/templates/dev/head'],
+        modules: [
+          'core/templates/dev/head',
+          'extensions'
+        ],
       },
       module: {
         rules: [{

diff --git a/...ns/rich_text_components/Collapsible/directives/OppiaNoninteractiveCollapsibleDirective.ts b/...ns/rich_text_components/Collapsible/directives/OppiaNoninteractiveCollapsibleDirective.ts
@@ -19,6 +19,10 @@
  * into the directive is: the name of the parameter, followed by 'With',
  * followed by the name of the arg.
  */
+
+require('domain/utilities/UrlInterpolationService.ts');
+require('services/HtmlEscaperService.ts');
+
 oppia.directive('oppiaNoninteractiveCollapsible', [
   '$rootScope', '$sce', 'HtmlEscaperService', 'UrlInterpolationService',
   function($rootScope, $sce, HtmlEscaperService, UrlInterpolationService) {

diff --git a/extensions/rich_text_components/Image/directives/OppiaNoninteractiveImageDirective.ts b/extensions/rich_text_components/Image/directives/OppiaNoninteractiveImageDirective.ts
@@ -19,6 +19,13 @@
  * into the directive is: the name of the parameter, followed by 'With',
  * followed by the name of the arg.
  */
+
+require('domain/utilities/UrlInterpolationService.ts');
+require('pages/exploration_player/ImagePreloaderService.ts');
+require('services/AssetsBackendApiService.ts');
+require('services/ContextService.ts');
+require('services/HtmlEscaperService.ts');
+
 oppia.directive('oppiaNoninteractiveImage', [
   '$rootScope', '$sce', 'AssetsBackendApiService', 'ContextService',
   'HtmlEscaperService', 'ImagePreloaderService',

diff --git a/extensions/rich_text_components/Link/directives/OppiaNoninteractiveLinkDirective.ts b/extensions/rich_text_components/Link/directives/OppiaNoninteractiveLinkDirective.ts
@@ -19,6 +19,11 @@
  * into the directive is: the name of the parameter, followed by 'With',
  * followed by the name of the arg.
  */
+
+require('domain/utilities/UrlInterpolationService.ts');
+require('services/ContextService.ts');
+require('services/HtmlEscaperService.ts');
+
 oppia.directive('oppiaNoninteractiveLink', [
   'HtmlEscaperService', 'UrlInterpolationService',
   function(HtmlEscaperService, UrlInterpolationService) {

diff --git a/extensions/rich_text_components/Math/directives/OppiaNoninteractiveMathDirective.ts b/extensions/rich_text_components/Math/directives/OppiaNoninteractiveMathDirective.ts
@@ -19,6 +19,10 @@
  * into the directive is: the name of the parameter, followed by 'With',
  * followed by the name of the arg.
  */
+
+require('domain/utilities/UrlInterpolationService.ts');
+require('services/HtmlEscaperService.ts');
+
 oppia.directive('oppiaNoninteractiveMath', [
   'HtmlEscaperService', 'UrlInterpolationService',
   function(HtmlEscaperService, UrlInterpolationService) {

diff --git a/extensions/rich_text_components/Tabs/directives/OppiaNoninteractiveTabsDirective.ts b/extensions/rich_text_components/Tabs/directives/OppiaNoninteractiveTabsDirective.ts
@@ -19,6 +19,10 @@
  * into the directive is: the name of the parameter, followed by 'With',
  * followed by the name of the arg.
  */
+
+require('domain/utilities/UrlInterpolationService.ts');
+require('services/HtmlEscaperService.ts');
+
 oppia.directive('oppiaNoninteractiveTabs', [
   'HtmlEscaperService', 'UrlInterpolationService',
   function(HtmlEscaperService, UrlInterpolationService) {

diff --git a/extensions/rich_text_components/Video/directives/OppiaNoninteractiveVideoDirective.ts b/extensions/rich_text_components/Video/directives/OppiaNoninteractiveVideoDirective.ts
@@ -19,6 +19,12 @@
  * into the directive is: the name of the parameter, followed by 'With',
  * followed by the name of the arg.
  */
+
+require('domain/utilities/UrlInterpolationService.ts');
+require('services/AutoplayedVideosService.ts');
+require('services/ContextService.ts');
+require('services/HtmlEscaperService.ts');
+
 oppia.directive('oppiaNoninteractiveVideo', [
   '$sce', 'HtmlEscaperService', 'UrlInterpolationService',
   function($sce, HtmlEscaperService, UrlInterpolationService) {

diff --git a/extensions/rich_text_components/richTextComponentsRequires.ts b/extensions/rich_text_components/richTextComponentsRequires.ts
@@ -0,0 +1,33 @@
+// Copyright 2019 The Oppia Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS-IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+/**
+ * @fileoverview Requires for all the RTC directives.
+ */
+
+require(
+  'rich_text_components/Collapsible/directives/' +
+  'OppiaNoninteractiveCollapsibleDirective.ts');
+require(
+  'rich_text_components/Image/directives/' +
+  'OppiaNoninteractiveImageDirective.ts');
+require(
+  'rich_text_components/Link/directives/OppiaNoninteractiveLinkDirective.ts');
+require(
+  'rich_text_components/Math/directives/OppiaNoninteractiveMathDirective.ts');
+require(
+  'rich_text_components/Tabs/directives/OppiaNoninteractiveTabsDirective.ts');
+require(
+  'rich_text_components/Video/directives/' +
+  'OppiaNoninteractiveVideoDirective.ts');
diff --git a/tsconfig.json b/tsconfig.json
@@ -7,7 +7,7 @@
     "rootDir": ".",
     "skipLibCheck": true,
     "target": "es5",
-    "typeRoots": ["./node_modules/@types"],
+    "typeRoots": ["./node_modules/@types"]
   },
   "files": [
     "assets/constants.js", 
@@ -21,5 +21,13 @@
     "core/templates/dev/head/filters/string-utility-filters/convert-to-plain-text.filter.ts",
     "core/templates/dev/head/filters/string-utility-filters/truncate-at-first-line.filter.ts"
   ],
-  "include": ["extensions", "typings"]
+  "include": [
+    "extensions/ckeditor_plugins", 
+    "extensions/classifiers",
+    "extensions/interactions",
+    "extensions/objects",
+    "extensions/value_generators",
+    "extensions/visualizations",
+    "typings"
+  ]
 }
diff --git a/webpack.dev.config.ts b/webpack.dev.config.ts
@@ -13,7 +13,7 @@
 // limitations under the License.
 
 /**
- * @fileoverview Dev environment config file for Webpack.
+ * @fileoverview Development environment config file for Webpack.
  */
 
 var commonWebpackConfig = require('./webpack.config.ts');
@@ -24,6 +24,7 @@ module.exports = {
   resolve: {
     modules: [
       path.resolve(__dirname, 'core/templates/dev/head'),
+      path.resolve(__dirname, 'extensions')
     ],
   },
   entry: commonWebpackConfig.entries,
@@ -33,7 +34,8 @@ module.exports = {
       test: /\.ts$/,
       include: [
         path.resolve(__dirname, 'core/templates/dev/head'),
-        path.resolve(__dirname, 'typings')
+        path.resolve(__dirname, 'extensions'),
+        path.resolve(__dirname, 'typings'),
       ],
       use: [
         'cache-loader',

diff --git a/webpack.prod.config.ts b/webpack.prod.config.ts
@@ -24,6 +24,7 @@ module.exports = {
   resolve: {
     modules: [
       path.resolve(__dirname, 'core/templates/dev/head'),
+      path.resolve(__dirname, 'extensions')
     ],
   },
   entry: commonWebpackConfig.entries,
@@ -33,7 +34,8 @@ module.exports = {
       test: /\.ts$/,
       include: [
         path.resolve(__dirname, 'core/templates/dev/head'),
-        path.resolve(__dirname, 'typings')
+        path.resolve(__dirname, 'extensions'),
+        path.resolve(__dirname, 'typings'),
       ],
       use: [
         'cache-loader',