Skip to content

Commit 060abf6

Browse files
committed
Use addParams where it would improve code
Summary: This commit replaces uses like `base + "?things"` (when `base` is not known to not already contain a query component), or that fail to call `encodeURIComponent` on the `things`, with uses of `addParams`. I generated this commit by running `git grep '[&?].*='` and investigating each occurrence. I ignored all occurrences in the graph and projector plugins, as they have their own mini-ecosystems. I also ignored the unique occurrence in the profile plugin, because (a) I can’t test it, and (b) its inputs are hard-coded such that this change provably would not fix any bugs. Test Plan: I verified the behavior of all existing plugins. wchargin-branch: use-add-params
1 parent 90afe93 commit 060abf6

File tree

5 files changed

+29
-36
lines changed

5 files changed

+29
-36
lines changed

tensorboard/components/tf_dashboard_common/tf-downloader.html

+2-1
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
</style>
7575
</template>
7676
<script>
77+
import {addParams} from "../tf-backend/urlPathHelpers.js";
7778
Polymer({
7879
is: "tf-downloader",
7980
properties: {
@@ -83,7 +84,7 @@
8384
urlFn: Function,
8485
},
8586
_csvUrl: function(_run, urlFn) {
86-
return urlFn(this.tag, _run) + "&format=csv";
87+
return addParams(urlFn(this.tag, _run), {format: "csv"});
8788
},
8889
_jsonUrl: function(_run, urlFn) {
8990
return urlFn(this.tag, _run);

tensorboard/plugins/audio/tf_audio_dashboard/tf-audio-loader.html

+10-15
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@
114114
</template>
115115
<script>
116116
"use strict";
117+
import {addParams} from "../tf-backend/urlPathHelpers.js";
117118
import {Canceller} from "../tf-backend/canceller.js";
118119
import {getRouter} from "../tf-backend/router.js";
119120
import {formatDate} from '../tf-card-heading/util.js';
@@ -210,9 +211,11 @@
210211
}
211212
this._metadataCanceller.cancelAll();
212213
const router = getRouter();
213-
const route = router.pluginRunTagRoute("audio", "/audio")(
214-
this.tag, this.run);
215-
const url = `${route}&sample=${this.sample}`;
214+
const url = addParams(router.pluginRoute('audio', '/audio'), {
215+
tag: this.tag,
216+
run: this.run,
217+
sample: this.sample,
218+
});
216219
const updateSteps = this._metadataCanceller.cancellable(result => {
217220
if (result.cancelled) {
218221
return;
@@ -225,27 +228,19 @@
225228
this.requestManager.request(url).then(updateSteps);
226229
},
227230
_createStepDatum(audioMetadata) {
228-
const route = getRouter().pluginRoute('audio', '/individualAudio');
229-
let query = audioMetadata.query;
230-
if (route.indexOf('?') > -1) {
231-
// The route already has GET parameters. Append ours.
232-
query = '&' + query;
233-
} else {
234-
// The route lacks GET parameters. Just use ours.
235-
query = '?' + query;
236-
}
237-
231+
let url = getRouter().pluginRoute('audio', '/individualAudio');
238232
// Include wall_time just to disambiguate the URL and force
239233
// the browser to reload the audio when the URL changes. The
240234
// backend doesn't care about the value.
241-
const individualAudioURL = `${route}${query}&ts=${audioMetadata.wall_time}`;
235+
url = addParams(url, {ts: audioMetadata.wall_time});
236+
url += '&' + audioMetadata.query;
242237

243238
return {
244239
wall_time: formatDate(new Date(audioMetadata.wall_time * 1000)),
245240
step: audioMetadata.step,
246241
label: audioMetadata.label,
247242
contentType: audioMetadata.contentType,
248-
url: individualAudioURL,
243+
url,
249244
};
250245
},
251246
});

tensorboard/plugins/image/tf_image_dashboard/tf-image-loader.html

+10-15
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@
181181
</template>
182182
<script>
183183
"use strict";
184+
import {addParams} from '../tf-backend/urlPathHelpers.js';
184185
import {Canceller} from "../tf-backend/canceller.js";
185186
import {getRouter} from "../tf-backend/router.js";
186187
import {formatDate} from '../tf-card-heading/util.js';
@@ -305,9 +306,11 @@
305306
}
306307
this._metadataCanceller.cancelAll();
307308
const router = getRouter();
308-
const route = router.pluginRunTagRoute("images", "/images")(
309-
this.tag, this.run);
310-
const url = `${route}&sample=${this.sample}`;
309+
const url = addParams(router.pluginRoute('images', '/images'), {
310+
tag: this.tag,
311+
run: this.run,
312+
sample: this.sample,
313+
});
311314
const updateSteps = this._metadataCanceller.cancellable(result => {
312315
if (result.cancelled) {
313316
return;
@@ -320,20 +323,12 @@
320323
this.requestManager.request(url).then(updateSteps);
321324
},
322325
_createStepDatum(imageMetadata) {
323-
const route = getRouter().pluginRoute('images', '/individualImage');
324-
let query = imageMetadata.query;
325-
if (route.indexOf('?') > -1) {
326-
// The route already has GET parameters. Append ours.
327-
query = '&' + query;
328-
} else {
329-
// The route lacks GET parameters. Just use ours.
330-
query = '?' + query;
331-
}
332-
326+
let url = getRouter().pluginRoute('images', '/individualImage');
333327
// Include wall_time just to disambiguate the URL and force
334328
// the browser to reload the image when the URL changes. The
335329
// backend doesn't care about the value.
336-
const individualImageUrl = `${route}${query}&ts=${imageMetadata.wall_time}`;
330+
url = addParams(url, {ts: imageMetadata.wall_time});
331+
url += '&' + imageMetadata.query;
337332

338333
return {
339334
width: imageMetadata.width,
@@ -342,7 +337,7 @@
342337
// constructor accepts a time in milliseconds, so we multiply by 1000.
343338
wall_time: new Date(imageMetadata.wall_time * 1000),
344339
step: imageMetadata.step,
345-
url: individualImageUrl,
340+
url,
346341
};
347342
},
348343
_updateImageUrl(steps, stepIndex) {

tensorboard/plugins/pr_curve/tf_pr_curve_dashboard/tf-pr-curve-card.html

+5-4
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@
154154
</style>
155155
</template>
156156
<script>
157+
import {addParams} from "../tf-backend/urlPathHelpers.js";
157158
import {Canceller} from "../tf-backend/canceller.js";
158159
import {getRouter} from "../tf-backend/router.js";
159160
import {runsColorScale} from "../tf-color-scale/colorScale.js";
@@ -293,10 +294,10 @@
293294
this._canceller.cancelAll();
294295
const router = getRouter();
295296

296-
let url = router.pluginRoute('pr_curves', '/pr_curves');
297-
url += url.indexOf('?') > -1 ? '&' : '?';
298-
url += `tag=${encodeURIComponent(this.tag)}`;
299-
url += this.runs.map(r => `&run=${encodeURIComponent(r)}`).join('');
297+
const url = addParams(router.pluginRoute('pr_curves', '/pr_curves'), {
298+
tag: this.tag,
299+
run: this.runs, // repeated URL parameter
300+
});
300301

301302
const updateData = this._canceller.cancellable(result => {
302303
if (result.cancelled) {

tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-chart.html

+2-1
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@
171171
</style>
172172
</template>
173173
<script>
174+
import {addParams} from '../tf-backend/urlPathHelpers.js';
174175
import {Canceller} from '../tf-backend/canceller.js';
175176
import {getRouter} from '../tf-backend/router.js';
176177
import {runsColorScale} from '../tf-color-scale/colorScale.js';
@@ -486,7 +487,7 @@
486487
},
487488

488489
_csvUrl(run) {
489-
return this._scalarUrl(this.tag, run) + '&format=csv';
490+
return addParams(this._scalarUrl(this.tag, run), {format: 'csv'});
490491
},
491492
_jsonUrl(run) {
492493
return this._scalarUrl(this.tag, run);

0 commit comments

Comments
 (0)