Skip to content

Commit e8a24c8

Browse files
bmd3kdna2github
authored andcommitted
projector: Fix esbuild-bundled projector. (tensorflow#5944)
The change to use the esbuild bundler (tensorflow#5829) broke the projector plugin (tensorflow#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.
1 parent eec07e9 commit e8a24c8

File tree

5 files changed

+47
-2
lines changed

5 files changed

+47
-2
lines changed

.github/workflows/ci.yml

+4
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,10 @@ jobs:
325325
! git grep -E '"@npm_angular_bazel//:index.bzl"' 'tensorboard/**/BUILD'
326326
- run: |
327327
! git grep -E 'mat-color|$mat-' 'tensorboard/**/*.scss'
328+
# Make sure no one depends on the numeric library directly. Please
329+
# import the indirection in //tensorboard/webapp/third_party.
330+
- run: |
331+
! git grep -E '@npm//numeric' 'tensorboard/*/BUILD' ':!tensorboard/webapp/third_party/**'
328332
329333
lint-misc: # build, protos, etc.
330334
runs-on: ubuntu-18.04

tensorboard/plugins/projector/vz_projector/BUILD

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ tf_ts_library(
6060
"//tensorboard/components/polymer:register_style_dom_module",
6161
"//tensorboard/components/tf_wbr_string",
6262
"//tensorboard/webapp/third_party:d3",
63+
"//tensorboard/webapp/third_party:numeric",
6364
"//tensorboard/webapp/third_party:tfjs",
6465
"@npm//@polymer/decorators",
6566
"@npm//@polymer/polymer",
66-
"@npm//numeric",
6767
"@npm//three",
6868
"@npm//umap-js",
6969
],

tensorboard/plugins/projector/vz_projector/data.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
See the License for the specific language governing permissions and
1313
limitations under the License.
1414
==============================================================================*/
15-
import numeric from 'numeric';
1615
import {UMAP} from 'umap-js';
16+
import {numeric} from '../../../webapp/third_party/numeric';
1717
import {TSNE} from './bh_tsne';
1818
import {SpriteMetadata} from './data-provider';
1919
import * as knn from './knn';

tensorboard/webapp/third_party/BUILD

+9
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,12 @@ tf_ts_library(
3131
"@npm//marked",
3232
],
3333
)
34+
35+
tf_ts_library(
36+
name = "numeric",
37+
srcs = ["numeric.ts"],
38+
strict_checks = False,
39+
deps = [
40+
"@npm//numeric",
41+
],
42+
)
+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/* Copyright 2022 The TensorFlow Authors. All Rights Reserved.
2+
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License.
14+
==============================================================================*/
15+
import numeric from 'numeric';
16+
17+
// The numeric library requires that the symbol 'numeric' is available in the
18+
// global scope when its operations are executed by other modules. See, for
19+
// example, how the definition of some of its operations refer to the string
20+
// 'numeric' in the Function definition, unmodifiable by the
21+
// bundler/minification code:
22+
//
23+
// https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696
24+
//
25+
// The esbuild bundler does not keep 'numeric' in global scope and instead
26+
// renames it as part of bundling/minification. We work around this by manually
27+
// adding it to global scope here.
28+
window['numeric'] = window['numeric'] ?? numeric;
29+
30+
// Reexport the numeric library. All imports of numeric should be done through
31+
// this file to ensure 'numeric' is available in the global scope.
32+
export {numeric};

0 commit comments

Comments
 (0)