From 4070ffc419baca9907c671a9190ce93b2e5e3b63 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 16 Jun 2022 18:49:24 +0100 Subject: [PATCH] feat(build): Support TypeScript in `core/` (#6220) * fix(tests): Use tsc-compiled base.js to allow use of goog.js The Closure Compiler complains if you try to feed it a file named goog.js which is not in the same directory as the Closure Library's base.js. Since tsc will "compile" goog.js when it encounters an "import ... from '.../goog.js'", it is necessary to also have tsc "compile" base.js and base_minimal.js, so they will come from the same directory. This necessitates some updates to paths in * docs(build): JSDoc update for JSCOMP_WARNING * refactor(utils): Convert utils/deprecation.js to TypeScript This was done manually for test/proving purposes and might need to be corrected based on what MigranTS generated. * chore(utils): Update utils/deprecation.ts from MigranTS output This manually applies certain changes from BeksOmega's ts/migration2 branch, but notably: - I did not apply the reordering of the doc comments at the top. - I applied the deletion of types and @package from the JSDoc. - I preserved the import goog and goog.declareModuleId lines. - I have applied a whitespace change on line 37 which violates the styleguide; I want to figure out why clang-format is not fixing this. * feat(build): clang-format .ts files And fix formatting issues introduced by MigranTS in deprecation.ts. * fix(build): Fix sources for advanced compilation test I'm not sure why this didn't fail on my local machine previously; perhaps it succeeded only because of leftover files and would have failed if I'd run npm run clean. * fix(build): Disable checkTypes diagnostic group Unfortunately TSC doesn't output type information in a form that Closure Compiler can understand, so the latter raises errors for situations like omitting an optional parameter. We may have to turn off more diagnostics in future, but for now this is sufficient. * chore(utils): Use @internal where we previously used @package Per comments on PR #6220. This requires that we disable the nonStandardJsDocs diagnostic. --- core/utils/{deprecation.js => deprecation.ts} | 22 +++---- scripts/gulpfiles/build_tasks.js | 59 +++++++++++++------ scripts/gulpfiles/chunks.json | 5 +- tests/bootstrap.js | 5 +- tsconfig.json | 2 +- 5 files changed, 60 insertions(+), 33 deletions(-) rename core/utils/{deprecation.js => deprecation.ts} (61%) diff --git a/core/utils/deprecation.js b/core/utils/deprecation.ts similarity index 61% rename from core/utils/deprecation.js rename to core/utils/deprecation.ts index 907322ae5..bd5731d47 100644 --- a/core/utils/deprecation.js +++ b/core/utils/deprecation.ts @@ -15,27 +15,29 @@ * This method is not specific to Blockly. * @namespace Blockly.utils.deprecation */ -goog.module('Blockly.utils.deprecation'); +import * as goog from '../../closure/goog/goog.js'; + +goog.declareModuleId('Blockly.utils.deprecation'); /** * Warn developers that a function or property is deprecated. - * @param {string} name The name of the function or property. - * @param {string} deprecationDate The date of deprecation. + * @param name The name of the function or property. + * @param deprecationDate The date of deprecation. * Prefer 'month yyyy' or 'quarter yyyy' format. - * @param {string} deletionDate The date of deletion, in the same format as the + * @param deletionDate The date of deletion, in the same format as the * deprecation date. - * @param {string=} opt_use The name of a function or property to use instead, - * if any. + * @param opt_use The name of a function or property to use instead, if any. * @alias Blockly.utils.deprecation.warn - * @package + * @internal */ -const warn = function(name, deprecationDate, deletionDate, opt_use) { +export function warn( + name: string, deprecationDate: string, deletionDate: string, + opt_use?: string) { let msg = name + ' was deprecated on ' + deprecationDate + ' and will be deleted on ' + deletionDate + '.'; if (opt_use) { msg += '\nUse ' + opt_use + ' instead.'; } console.warn(msg); -}; -exports.warn = warn; +} diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index ce13aec18..3e709f940 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -174,7 +174,7 @@ var JSCOMP_ERROR = [ // 'accessControls', // Deprecated; means same as visibility. 'checkPrototypalTypes', 'checkRegExp', - 'checkTypes', + // 'checkTypes', // Disabled; see note in JSCOMP_OFF. 'checkVars', 'conformanceViolations', 'const', @@ -198,7 +198,7 @@ var JSCOMP_ERROR = [ // 'missingSourcesWarnings', // Group of several other options. 'moduleLoad', 'msgDescriptions', - 'nonStandardJsDocs', + // 'nonStandardJsDocs', // Disabled; see note in JSCOMP_OFF. // 'partialAlias', // Don't want this to be an error yet; only warning. // 'polymer', // Not applicable. // 'reportUnknownTypes', // VERY verbose. @@ -222,15 +222,36 @@ var JSCOMP_ERROR = [ /** * Closure compiler diagnostic groups we want to be treated as warnings. * These are effected when the --debug or --strict flags are passed. + * + * For most (all?) diagnostic groups this is the default level, so + * it's generally sufficient to remove them from JSCOMP_ERROR. */ var JSCOMP_WARNING = [ ]; /** - * Closure compiler diagnostic groups we want to be ignored. - * These suppressions are always effected by default. + * Closure compiler diagnostic groups we want to be ignored. These + * suppressions are always effected by default. + * + * Make sure that anything added here is commented out of JSCOMP_ERROR + * above, as that takes precedence.) */ var JSCOMP_OFF = [ + /* The removal of Closure type system types from our JSDoc + * annotations means that the closure compiler now generates certain + * diagnostics because it no longer has enough information to be + * sure that the input code is correct. The following diagnostic + * groups are turned off to suppress such errors. + * + * When adding additional items to this list it may be helpful to + * search the compiler source code + * (https://github.com/google/closure-compiler/) for the JSC_* + * disagnostic name (omitting the JSC_ prefix) to find the corresponding + * DiagnosticGroup. + */ + 'checkTypes', + 'nonStandardJsDocs', // Due to @internal + /* In order to transition to ES modules, modules will need to import * one another by relative paths. This means that the previous * practice of moving all source files into the same directory for @@ -269,12 +290,8 @@ function buildJavaScript(done) { * suite. */ function buildDeps(done) { - const closurePath = argv.closureLibrary ? - 'node_modules/google-closure-library/closure/goog' : - 'closure/goog'; - const roots = [ - closurePath, + path.join(TSC_OUTPUT_DIR, 'closure', 'goog', 'base.js'), TSC_OUTPUT_DIR, 'blocks', 'generators', @@ -430,15 +447,15 @@ return ${exportsExpression}; * @return {{chunk: !Array, js: !Array}} The chunking * information, in the same form as emitted by * closure-calculate-chunks. - * - * TODO(cpcallen): maybeAddClosureLibrary? Or maybe remove base.js? */ function getChunkOptions() { if (argv.compileTs) { chunks[0].entry = path.join(TSC_OUTPUT_DIR, chunks[0].entry); } + const basePath = + path.join(TSC_OUTPUT_DIR, 'closure', 'goog', 'base_minimal.js'); const cccArgs = [ - '--closure-library-base-js-path ./closure/goog/base_minimal.js', + `--closure-library-base-js-path ./${basePath}`, `--deps-file './${DEPS_FILE}'`, ...(chunks.map(chunk => `--entrypoint '${chunk.entry}'`)), ]; @@ -545,7 +562,11 @@ function compile(options) { language_out: 'ECMASCRIPT5_STRICT', jscomp_off: [...JSCOMP_OFF], rewrite_polyfills: true, - hide_warnings_for: 'node_modules', + // N.B.: goog.js refers to lots of properties on goog that are not + // declared by base_minimal.js, while if you compile against + // base.js instead you will discover that it uses @deprecated + // inherits, forwardDeclare etc. + hide_warnings_for: ['node_modules', 'build/src/closure/goog/goog.js'], define: ['COMPILED=true'], }; if (argv.debug || argv.strict) { @@ -597,11 +618,10 @@ function buildCompiled() { * closure compiler's ADVANCED_COMPILATION mode. */ function buildAdvancedCompilationTest() { - const coreSrcs = argv.compileTs ? - TSC_OUTPUT_DIR + '/core/**/*.js' : 'core/**/*.js'; const srcs = [ - 'closure/goog/base_minimal.js', - coreSrcs, + TSC_OUTPUT_DIR + '/closure/goog/base_minimal.js', + TSC_OUTPUT_DIR + '/closure/goog/goog.js', + TSC_OUTPUT_DIR + '/core/**/*.js', 'blocks/**/*.js', 'generators/**/*.js', 'tests/compile/main.js', @@ -668,7 +688,10 @@ function cleanBuildDir(done) { * Runs clang format on all files in the core directory. */ function format() { - return gulp.src(['core/**/*.js', 'blocks/**/*.js'], {base: '.'}) + return gulp.src([ + 'core/**/*.js', 'core/**/*.ts', + 'blocks/**/*.js', 'blocks/**/*.ts' + ], {base: '.'}) .pipe(clangFormatter.format('file', clangFormat)) .pipe(gulp.dest('.')); }; diff --git a/scripts/gulpfiles/chunks.json b/scripts/gulpfiles/chunks.json index 0ad47025d..d35ed7089 100644 --- a/scripts/gulpfiles/chunks.json +++ b/scripts/gulpfiles/chunks.json @@ -1,6 +1,6 @@ { "chunk": [ - "blockly:259", + "blockly:260", "blocks:10:blockly", "all:11:blockly", "all1:11:blockly", @@ -250,6 +250,7 @@ "./build/src/core/utils/parsing.js", "./build/src/core/extensions.js", "./build/src/core/block.js", + "./build/src/closure/goog/goog.js", "./build/src/core/utils/deprecation.js", "./build/src/core/utils/string.js", "./build/src/core/dialog.js", @@ -266,7 +267,7 @@ "./build/src/core/connection.js", "./build/src/core/common.js", "./build/src/core/blocks.js", - "./closure/goog/base_minimal.js", + "./build/src/closure/goog/base_minimal.js", "./build/src/core/blockly.js", "./blocks/variables_dynamic.js", "./blocks/variables.js", diff --git a/tests/bootstrap.js b/tests/bootstrap.js index 45f64bb19..15c207c63 100644 --- a/tests/bootstrap.js +++ b/tests/bootstrap.js @@ -134,7 +134,8 @@ // libary we use, mainly for goog.require / goog.provide / // goog.module). document.write( - ''); + ''); // Prevent spurious transpilation warnings. document.write(''); @@ -174,7 +175,7 @@ for (let script, i = 0; script = scripts[i]; i++) { const fakeModuleName = 'script.' + script.replace(/[./]/g, "-"); scriptDeps.push(' goog.addDependency(' + - quote('../../' + script) + ', [' + quote(fakeModuleName) + + quote('../../../../' + script) + ', [' + quote(fakeModuleName) + '], [' + requires.map(quote).join() + "], {'lang': 'es6'});\n"); requires = [fakeModuleName]; } diff --git a/tsconfig.json b/tsconfig.json index 12a91c8b0..f167444b3 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,7 +1,7 @@ { "include": [ "core/**/*", // N.B.: also pulls in closure/goog/goog.js if needed. - "closure/goog/goog.js", // Just for ouptut directory structure. + "closure/**/*", // Just for ouptut directory structure. ], "compilerOptions": { // Tells TypeScript to read JS files, as