Skip to content

Commit 71f68bf

Browse files
authored
Fixes path_prefix flag (#1623)
PR #1512 introduced a bug where it formed a complete path without using existing pathname. Previously without forming the complete path, it was treated as a relative path from current window.location.pathname. We decided to revert back to the old behavior without using `new URL` which was not providing a clear benefit.
1 parent a0bbb30 commit 71f68bf

File tree

2 files changed

+140
-118
lines changed

2 files changed

+140
-118
lines changed

tensorboard/components/tf_backend/router.ts

Lines changed: 47 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ export interface Router {
1818
environment: () => string;
1919
experiments: () => string;
2020
isDemoMode: () => boolean;
21-
pluginRoute: (pluginName: string, route: string) => string;
21+
pluginRoute: (pluginName: string, route: string,
22+
params?: URLSearchParams, demoCustomExt?: string) => string;
2223
pluginsListing: () => string;
2324
runs: () => string;
2425
runsForExperiment: (id: tf_backend.ExperimentId) => string;
@@ -28,33 +29,33 @@ export interface Router {
2829
* Create a router for communicating with the TensorBoard backend. You
2930
* can pass this to `setRouter` to make it the global router.
3031
*
31-
* @param pathPrefix {string=} The base prefix for data endpoints.
32+
* @param dataDir {string=} The base prefix for data endpoints.
3233
* @param demoMode {boolean=} Whether to modify urls for filesystem demo usage.
3334
*/
34-
export function createRouter(pathPrefix = 'data', demoMode = false): Router {
35-
if (pathPrefix[pathPrefix.length - 1] === '/') {
36-
pathPrefix = pathPrefix.slice(0, pathPrefix.length - 1);
35+
export function createRouter(dataDir = 'data', demoMode = false): Router {
36+
if (dataDir[dataDir.length - 1] === '/') {
37+
dataDir = dataDir.slice(0, dataDir.length - 1);
3738
}
38-
const createPath = demoMode ? createDemoPath : createProdPath;
39+
const createDataPath = demoMode ? createDemoDataPath : createProdDataPath;
3940
const ext = demoMode ? '.json' : '';
4041
return {
41-
environment: () => createPath(pathPrefix, '/environment', ext),
42-
experiments: () => createPath(pathPrefix, '/experiments', ext),
42+
environment: () => createDataPath(dataDir, '/environment', ext),
43+
experiments: () => createDataPath(dataDir, '/experiments', ext),
4344
isDemoMode: () => demoMode,
4445
pluginRoute: (pluginName: string, route: string,
4546
params?: URLSearchParams, demoCustomExt = ext): string => {
4647

47-
return createPath(
48-
demoMode ? pathPrefix : pathPrefix + '/plugin',
48+
return createDataPath(
49+
demoMode ? dataDir : dataDir + '/plugin',
4950
`/${pluginName}${route}`,
5051
demoCustomExt,
5152
params);
5253
},
53-
pluginsListing: () => createPath(pathPrefix, '/plugins_listing', ext),
54-
runs: () => createPath(pathPrefix, '/runs', ext),
54+
pluginsListing: () => createDataPath(dataDir, '/plugins_listing', ext),
55+
runs: () => createDataPath(dataDir, '/runs', ext),
5556
runsForExperiment: id => {
56-
return createPath(
57-
pathPrefix,
57+
return createDataPath(
58+
dataDir,
5859
'/experiment_runs',
5960
ext,
6061
createSearchParam({experiment: String(id)}));
@@ -86,42 +87,55 @@ export function setRouter(router: Router): void {
8687
_router = router;
8788
}
8889

89-
function createProdPath(pathPrefix: string, path: string,
90-
ext: string, params?: URLSearchParams): string {
91-
const url = new URL(window.location.origin);
92-
// Use URL to normalize pathPrefix with leading slash and without.
93-
url.pathname = pathPrefix + path;
94-
if (params) url.search = params.toString();
95-
return url.pathname + url.search;
90+
function createProdDataPath(dataDir: string, route: string,
91+
ext: string, params: URLSearchParams = new URLSearchParams()): string {
92+
let relativePath = dataDir + route;
93+
if (String(params)) {
94+
const delimiter = route.includes('?') ? '&' : '?';
95+
relativePath += delimiter + String(params);
96+
}
97+
return relativePath;
9698
}
9799

98100
/**
99-
* Creates a URL for demo.
101+
* Creates a URL for demo apps.
102+
*
103+
* [1]: Demo pages are served as files and data routes are served as JSON files.
104+
* For shareability and ease of use, the data files are served at root[2], "/",
105+
* thus, the demo data path should return the absolute path regardless of
106+
* current pathname.
107+
*
108+
* [2]: See the path property of tensorboard/demo/BUILD:demo_data.
109+
*
100110
* e.g.,
101-
* > createDemoPath('a', '/b', '.json', {a: 1})
111+
* > createDemoDataPath('a', '/b', '.json', {a: 1})
102112
* < '/a/b_a_1.json'
103113
*/
104-
function createDemoPath(pathPrefix: string, path: string,
105-
ext: string, params?: URLSearchParams): string {
114+
function createDemoDataPath(dataDir: string, route: string,
115+
ext: string, params: URLSearchParams = new URLSearchParams()): string {
106116

107117
// First, parse the path in a safe manner by constructing a URL. We don't
108118
// trust the path supplied by consumer.
109-
const prefixLessUrl = new URL(`${window.location.origin}/${path}`);
110-
let {pathname: normalizedPath} = prefixLessUrl;
111-
const encodedQueryParam = params ?
112-
params.toString().replace(/[&=%]/g, '_') : '';
119+
const absRoute = route.startsWith('/') ? route : '/' + route;
120+
const absUrl = new URL(route, window.location.href);
121+
let {pathname: normalizedPath, searchParams: normalizedSearchParams} = absUrl;
122+
const queryParam = [normalizedSearchParams, params]
123+
.map(p => String(p))
124+
.filter(Boolean)
125+
.join('&');
126+
const encodedQueryParam = queryParam.replace(/[&=%]/g, '_');
113127

114128
// Strip leading slashes.
115129
normalizedPath = normalizedPath.replace(/^\/+/g, '');
116130
// Convert slashes to underscores.
117131
normalizedPath = normalizedPath.replace(/\//g, '_');
118132
// Add query parameter as path if it is present.
119133
if (encodedQueryParam) normalizedPath += `_${encodedQueryParam}`;
120-
const url = new URL(window.location.origin);
121134

122-
// All demo data are serialized in JSON format.
123-
url.pathname = `${pathPrefix}/${normalizedPath}${ext}`;
124-
return url.pathname + url.search;
135+
const pathname = `${dataDir}/${normalizedPath}${ext}`;
136+
// See [1] for the reason why we are forming an absolute path here.
137+
const absPathname = pathname.startsWith('/') ? pathname : '/' + pathname;
138+
return absPathname;
125139
}
126140

127141
export function createSearchParam(params: QueryParams = {}): URLSearchParams {

0 commit comments

Comments
 (0)