Skip to content

Commit

Permalink
feat(build): Support TypeScript in core/ (#6220)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
cpcallen authored Jun 16, 2022
1 parent e6a0b0c commit 4070ffc
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 33 deletions.
22 changes: 12 additions & 10 deletions core/utils/deprecation.js → core/utils/deprecation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
59 changes: 41 additions & 18 deletions scripts/gulpfiles/build_tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -430,15 +447,15 @@ return ${exportsExpression};
* @return {{chunk: !Array<string>, js: !Array<string>}} 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}'`)),
];
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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('.'));
};
Expand Down
5 changes: 3 additions & 2 deletions scripts/gulpfiles/chunks.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"chunk": [
"blockly:259",
"blockly:260",
"blocks:10:blockly",
"all:11:blockly",
"all1:11:blockly",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions tests/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@
// libary we use, mainly for goog.require / goog.provide /
// goog.module).
document.write(
'<script src="' + options.root + '/closure/goog/base.js"></script>');
'<script src="' + options.root +
'build/src/closure/goog/base.js"></script>');

// Prevent spurious transpilation warnings.
document.write('<script>goog.TRANSPILE = "never";</script>');
Expand Down Expand Up @@ -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];
}
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
@@ -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
Expand Down

0 comments on commit 4070ffc

Please sign in to comment.