From 229db27a7e8d5459566c944776afd4c1b7250b4d Mon Sep 17 00:00:00 2001 From: Brian Dubois Date: Thu, 22 Sep 2022 14:08:37 -0400 Subject: [PATCH] projector: Fix esbuild-bundled projector. (#5944) The change to use the esbuild bundler (https://github.com/tensorflow/tensorboard/pull/5829) broke the projector plugin (https://github.com/tensorflow/tensorboard/issues/5924). The root cause is in the 'numeric' library, which we use for calculating the PCA. The library requires that the symbol 'numeric' is available in the global scope when its operations are executed by other modules. See, for example, how the definition of some of its operations refer to the string 'numeric' in the Function definition, unmodifiable by the bundler/minification code: https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696 The esbuild bundler does not keep 'numeric' in global scope and instead renames it as part of bundling/minification. We work around this by manually adding it to global scope. We introduce the wrapper tensorboard/webapp/third_party/numeric.ts to add 'numeric' to the global scope and require that tensorboard code import that wrapper rather than importing numeric directly. We add a CI check to ensure that numeric is not imported directly. --- .github/workflows/ci.yml | 4 +++ .../plugins/projector/vz_projector/BUILD | 2 +- .../plugins/projector/vz_projector/data.ts | 2 +- tensorboard/webapp/third_party/BUILD | 9 ++++++ tensorboard/webapp/third_party/numeric.ts | 32 +++++++++++++++++++ 5 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tensorboard/webapp/third_party/numeric.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ebd656adb8e..f960b6fb524 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -325,6 +325,10 @@ jobs: ! git grep -E '"@npm_angular_bazel//:index.bzl"' 'tensorboard/**/BUILD' - run: | ! git grep -E 'mat-color|$mat-' 'tensorboard/**/*.scss' + # Make sure no one depends on the numeric library directly. Please + # import the indirection in //tensorboard/webapp/third_party. + - run: | + ! git grep -E '@npm//numeric' 'tensorboard/*/BUILD' ':!tensorboard/webapp/third_party/**' lint-misc: # build, protos, etc. runs-on: ubuntu-18.04 diff --git a/tensorboard/plugins/projector/vz_projector/BUILD b/tensorboard/plugins/projector/vz_projector/BUILD index 169f99e50a4..096724f29f2 100644 --- a/tensorboard/plugins/projector/vz_projector/BUILD +++ b/tensorboard/plugins/projector/vz_projector/BUILD @@ -60,10 +60,10 @@ tf_ts_library( "//tensorboard/components/polymer:register_style_dom_module", "//tensorboard/components/tf_wbr_string", "//tensorboard/webapp/third_party:d3", + "//tensorboard/webapp/third_party:numeric", "//tensorboard/webapp/third_party:tfjs", "@npm//@polymer/decorators", "@npm//@polymer/polymer", - "@npm//numeric", "@npm//three", "@npm//umap-js", ], diff --git a/tensorboard/plugins/projector/vz_projector/data.ts b/tensorboard/plugins/projector/vz_projector/data.ts index c20590aeed7..5a7d96139ca 100644 --- a/tensorboard/plugins/projector/vz_projector/data.ts +++ b/tensorboard/plugins/projector/vz_projector/data.ts @@ -12,8 +12,8 @@ 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. ==============================================================================*/ -import numeric from 'numeric'; import {UMAP} from 'umap-js'; +import {numeric} from '../../../webapp/third_party/numeric'; import {TSNE} from './bh_tsne'; import {SpriteMetadata} from './data-provider'; import * as knn from './knn'; diff --git a/tensorboard/webapp/third_party/BUILD b/tensorboard/webapp/third_party/BUILD index d4a3ce1b7b6..4e2e76cecfa 100644 --- a/tensorboard/webapp/third_party/BUILD +++ b/tensorboard/webapp/third_party/BUILD @@ -31,3 +31,12 @@ tf_ts_library( "@npm//marked", ], ) + +tf_ts_library( + name = "numeric", + srcs = ["numeric.ts"], + strict_checks = False, + deps = [ + "@npm//numeric", + ], +) diff --git a/tensorboard/webapp/third_party/numeric.ts b/tensorboard/webapp/third_party/numeric.ts new file mode 100644 index 00000000000..95f2c9efac3 --- /dev/null +++ b/tensorboard/webapp/third_party/numeric.ts @@ -0,0 +1,32 @@ +/* Copyright 2022 The TensorFlow 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. +==============================================================================*/ +import numeric from 'numeric'; + +// The numeric library requires that the symbol 'numeric' is available in the +// global scope when its operations are executed by other modules. See, for +// example, how the definition of some of its operations refer to the string +// 'numeric' in the Function definition, unmodifiable by the +// bundler/minification code: +// +// https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696 +// +// The esbuild bundler does not keep 'numeric' in global scope and instead +// renames it as part of bundling/minification. We work around this by manually +// adding it to global scope here. +window['numeric'] = window['numeric'] ?? numeric; + +// Reexport the numeric library. All imports of numeric should be done through +// this file to ensure 'numeric' is available in the global scope. +export {numeric};