Reland "Move `UIString` into `Platform`"
This is a reland of 992295e37d43a661098e7c4dd50e65aa765fdb1f
Original change's description:
> Move `UIString` into `Platform`
>
> Platform is the "lowest" part of DevTools - e.g. it depends on nothing.
>
> We have code we want to move to Platform but cannot because it needs
> `ls` and therefore creates a circular dependency. So this CL moves
> UIString into Platform.
>
> To avoid rewriting a lot of imports we re-export it from `common.js`.
>
> Change-Id: I399bbe8593e043148c4c7e51f598cbd73a89f639
> Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2165758
> Commit-Queue: Jack Franklin <[email protected]>
> Reviewed-by: Tim van der Lippe <[email protected]>
> Reviewed-by: Paul Lewis <[email protected]>
Change-Id: I482323d1b35eb38066a571cb8630078b6cff6cde
[email protected],[email protected],[email protected]
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2165793
Reviewed-by: Mathias Bynens <[email protected]>
Commit-Queue: Mathias Bynens <[email protected]>
diff --git a/BUILD.gn b/BUILD.gn
index 1aefe11..879dc00 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1115,7 +1115,6 @@
"common/TextDictionary.js",
"common/Throttler.js",
"common/Trie.js",
- "common/UIString.js",
"common/Worker.js",
"components/DockController.js",
"components/ImagePreview.js",
@@ -1329,6 +1328,7 @@
"platform/string-utilities.js",
"platform/number-utilities.js",
"platform/array-utilities.js",
+ "platform/UIString.js",
"profiler/TopDownProfileDataGrid.js",
"profiler/ProfileView.js",
"profiler/ProfileTypeRegistry.js",
diff --git a/front_end/common/BUILD.gn b/front_end/common/BUILD.gn
index 2f61932..5a3b118 100644
--- a/front_end/common/BUILD.gn
+++ b/front_end/common/BUILD.gn
@@ -27,7 +27,6 @@
"TextDictionary.js",
"Throttler.js",
"Trie.js",
- "UIString.js",
"Worker.js",
"common.js",
]
diff --git a/front_end/common/ResourceType.js b/front_end/common/ResourceType.js
index 3cce9a0..3fa8f7c 100644
--- a/front_end/common/ResourceType.js
+++ b/front_end/common/ResourceType.js
@@ -27,8 +27,9 @@
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
+import {ls} from '../platform/platform.js';
+
import {ParsedURL} from './ParsedURL.js';
-import {ls} from './UIString.js';
/**
* @unrestricted
diff --git a/front_end/common/common.js b/front_end/common/common.js
index 93101cb..ddfe1df 100644
--- a/front_end/common/common.js
+++ b/front_end/common/common.js
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-import '../platform/platform.js';
+import * as Platform from '../platform/platform.js';
import * as App from './App.js';
import * as AppProvider from './AppProvider.js';
@@ -25,10 +25,16 @@
import * as TextDictionary from './TextDictionary.js';
import * as Throttler from './Throttler.js';
import * as Trie from './Trie.js';
-import * as UIString from './UIString.js';
import * as Worker from './Worker.js';
-export const ls = UIString.ls;
+/* This is re-exported here because we moved UIString into platform from
+ * common and wanted to avoid a huge rename of imports. A future CL will
+ * update all references to `Common.UIString` to `Platform.UIString`.
+ */
+export {UIString} from '../platform/platform.js';
+
+export const ls = Platform.UIString.ls;
+
/**
* @type {!Settings.Settings}
@@ -57,6 +63,5 @@
TextDictionary,
Throttler,
Trie,
- UIString,
Worker,
};
diff --git a/front_end/common/ls.ts b/front_end/common/ls.ts
deleted file mode 100644
index 57bf1fd..0000000
--- a/front_end/common/ls.ts
+++ /dev/null
@@ -1,14 +0,0 @@
-// Copyright 2020 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-/* We want to be able to use ls for TypeScript land and make it really
- * easy to import e.g. we want to be able to call ls`foo` in a TS file
- * and not Common.UIString.ls`foo` so it's a bit easier and less verbose.
- * At the time of writing all of Common hasn't been TypeScriptified so
- * we create this import-and-export so from TypeScript we can import the
- * file directly via: import { ls } from '../common/ls.js'; which is
- * exempt from the ESLint module importing rules (see
- * rulesdir/es_modules_import for code + test case).
- */
-export {ls} from './UIString.js';
diff --git a/front_end/common/module.json b/front_end/common/module.json
index a3c96b6..11cbbf1 100644
--- a/front_end/common/module.json
+++ b/front_end/common/module.json
@@ -13,7 +13,6 @@
"Console.js",
"ParsedURL.js",
"Progress.js",
- "UIString.js",
"ResourceType.js",
"Settings.js",
"SegmentedRange.js",
diff --git a/front_end/platform/BUILD.gn b/front_end/platform/BUILD.gn
index 9370e5b..6df9be0 100644
--- a/front_end/platform/BUILD.gn
+++ b/front_end/platform/BUILD.gn
@@ -6,6 +6,7 @@
ts_library("platform") {
sources = [
+ "UIString.js",
"array-utilities.js",
"number-utilities.js",
"platform.js",
diff --git a/front_end/common/UIString.js b/front_end/platform/UIString.js
similarity index 84%
rename from front_end/common/UIString.js
rename to front_end/platform/UIString.js
index bec475a..8de91b5 100644
--- a/front_end/common/UIString.js
+++ b/front_end/platform/UIString.js
@@ -29,7 +29,7 @@
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-import * as Platform from '../platform/platform.js';
+import * as StringUtilities from './string-utilities.js';
/**
* @param {string} string
@@ -37,7 +37,7 @@
* @return {string}
*/
export function UIString(string, ...vararg) {
- return Platform.StringUtilities.vsprintf(localize(string), Array.prototype.slice.call(arguments, 1));
+ return StringUtilities.vsprintf(localize(string), Array.prototype.slice.call(arguments, 1));
}
/**
@@ -82,8 +82,8 @@
/** @type {string} */
this._localizedFormat = localize(format);
/** @type {!Array.<!Object>} */
- this._tokenizedFormat = Platform.StringUtilities.tokenizeFormatString(
- this._localizedFormat, Platform.StringUtilities.standardFormatters);
+ this._tokenizedFormat =
+ StringUtilities.tokenizeFormatString(this._localizedFormat, StringUtilities.standardFormatters);
}
/**
@@ -100,12 +100,12 @@
* @return {string}
*/
format(vararg) {
- return Platform.StringUtilities
+ return StringUtilities
.format(
- // the code here uses odd generics that Closure likes but TS doesn't
- // so rather than fight to typecheck this in a dodgy way we just let TS ignore it
- // @ts-ignore
- this._localizedFormat, arguments, Platform.StringUtilities.standardFormatters, '', UIStringFormat._append,
+ // the code here uses odd generics that Closure likes but TS doesn't
+ // so rather than fight to typecheck this in a dodgy way we just let TS ignore it
+ // @ts-ignore
+ this._localizedFormat, arguments, StringUtilities.standardFormatters, '', UIStringFormat._append,
this._tokenizedFormat)
.formattedResult;
}
diff --git a/front_end/platform/module.json b/front_end/platform/module.json
index c97647f..fe3a377 100644
--- a/front_end/platform/module.json
+++ b/front_end/platform/module.json
@@ -6,6 +6,7 @@
"utilities.js",
"string-utilities.js",
"number-utilities.js",
- "array-utilities.js"
+ "array-utilities.js",
+ "UIString.js"
]
}
diff --git a/front_end/platform/platform.js b/front_end/platform/platform.js
index 10e9550..c1ef7d6 100644
--- a/front_end/platform/platform.js
+++ b/front_end/platform/platform.js
@@ -33,6 +33,12 @@
import * as ArrayUtilities from './array-utilities.js';
import * as NumberUtilities from './number-utilities.js';
import * as StringUtilities from './string-utilities.js';
+import * as UIString from './UIString.js';
+
+/* We expose `ls` directly to make importing + referring to it easy
+ * as it's such a commonly referenced and used utility.
+ */
+export const {ls} = UIString;
export {Multimap} from './utilities.js';
-export {ArrayUtilities, NumberUtilities, StringUtilities};
+export {ArrayUtilities, NumberUtilities, StringUtilities, UIString};
diff --git a/scripts/eslint_rules/lib/check_license_header.js b/scripts/eslint_rules/lib/check_license_header.js
index a1e6a69..4576c1f 100644
--- a/scripts/eslint_rules/lib/check_license_header.js
+++ b/scripts/eslint_rules/lib/check_license_header.js
@@ -128,7 +128,6 @@
// IBM Corp
'sources/WatchExpressionsSidebarPane.js',
// Multiple authors
- 'common/UIString.js',
'components/JSPresentationUtils.js',
'console/ConsoleView.js',
'console/ConsoleViewMessage.js',
@@ -150,6 +149,7 @@
'object_ui/ObjectPropertiesSection.js',
'perf_ui/TimelineGrid.js',
'platform/utilities.js',
+ 'platform/UIString.js',
'resources/ApplicationPanelSidebar.js',
'resources/CookieItemsView.js',
'resources/DOMStorageItemsView.js',
diff --git a/scripts/eslint_rules/lib/es_modules_import.js b/scripts/eslint_rules/lib/es_modules_import.js
index 42623dc..4d59d49 100644
--- a/scripts/eslint_rules/lib/es_modules_import.js
+++ b/scripts/eslint_rules/lib/es_modules_import.js
@@ -62,6 +62,10 @@
}
}
+function nodeSpecifiersImportLsOnly(specifiers) {
+ return specifiers.length === 1 && specifiers[0].type === 'ImportSpecifier' && specifiers[0].imported.name === 'ls';
+}
+
module.exports = {
meta: {
type: 'problem',
@@ -114,10 +118,8 @@
return;
}
- if (importPath.endsWith(path.join('common', 'ls.js')) && path.extname(importingFileName) === '.ts') {
- /* We allow TypeScript files to import the ls module directly.
- * See common/ls.ts for more detail.
- */
+ if (importPath.endsWith(path.join('platform', 'platform.js')) && nodeSpecifiersImportLsOnly(node.specifiers)) {
+ /* We allow direct importing of the ls utility as it's so frequently used. */
return;
}
diff --git a/scripts/eslint_rules/tests/es_modules_import_test.js b/scripts/eslint_rules/tests/es_modules_import_test.js
index 8cb2277..a650596 100644
--- a/scripts/eslint_rules/tests/es_modules_import_test.js
+++ b/scripts/eslint_rules/tests/es_modules_import_test.js
@@ -54,9 +54,9 @@
code: 'import {appendStyle} from \'./append-style.js\';',
filename: 'front_end/ui/utils/utils.js',
},
- // the `ls` helper is an exception in a TypeScript file
+ // the `ls` helper from Platform is an exception
{
- code: 'import {ls} from \'../common/ls.js\';',
+ code: 'import {ls} from \'../platform/platform.js\';',
filename: 'front_end/elements/ElementsBreadcrumbs.ts',
},
// lit-html is exempt from any rules
@@ -134,15 +134,6 @@
}],
output: 'export {UIString} from \'../platform/platform.js\';'
},
- // the `ls` helper is not an exception in a JS file
- {
- code: 'import {ls} from \'../common/ls.js\';',
- filename: 'front_end/elements/ElementsPanel.js',
- errors: [{
- message:
- 'Incorrect cross-namespace import: "../common/ls.js". Use "import * as Namespace from \'../namespace/namespace.js\';" instead. You may only import common/ls.js directly from TypeScript source files.'
- }],
- },
// third-party modules are not exempt by default
{
code: 'import {someThing} from \'../third_party/some-module/foo.js\';',
diff --git a/test/unittests/front_end/common/BUILD.gn b/test/unittests/front_end/common/BUILD.gn
index fe94e0e..cdf2a49 100644
--- a/test/unittests/front_end/common/BUILD.gn
+++ b/test/unittests/front_end/common/BUILD.gn
@@ -16,7 +16,6 @@
"TextDictionary_test.ts",
"Throttler_test.ts",
"Trie_test.ts",
- "UIString_test.ts",
]
deps = [ "../../../../front_end/common" ]
diff --git a/test/unittests/front_end/platform/BUILD.gn b/test/unittests/front_end/platform/BUILD.gn
index 686dc8b..3d3cc10 100644
--- a/test/unittests/front_end/platform/BUILD.gn
+++ b/test/unittests/front_end/platform/BUILD.gn
@@ -6,6 +6,7 @@
"ArrayUtilities_test.ts",
"NumberUtilities_test.ts",
"StringUtilities_test.ts",
+ "UIString_test.ts",
"Utilities_test.ts",
]
diff --git a/test/unittests/front_end/common/UIString_test.ts b/test/unittests/front_end/platform/UIString_test.ts
similarity index 94%
rename from test/unittests/front_end/common/UIString_test.ts
rename to test/unittests/front_end/platform/UIString_test.ts
index a215d56..0131b91 100644
--- a/test/unittests/front_end/common/UIString_test.ts
+++ b/test/unittests/front_end/platform/UIString_test.ts
@@ -4,7 +4,7 @@
const {assert} = chai;
-import * as UIString from '../../../../front_end/common/UIString.js';
+import * as UIString from '../../../../front_end/platform/UIString.js';
describe('UIString', () => {
it('serializes UI strings', () => {
