2017131 - Reverse the key/values of BrowserSearchTelemetry.KNOWN_SEARCH_SOURCES and improve related TypeScript information

Status: REOPENED

(Firefox :: Search, task, P2)

Description

[Mark Banner (:standard8)]

BrowserSearchTelemetry.KNOWN_SEARCH_SOURCES is currently a map of the legacy names for SAP sources to their current names. Some of the legacy names use dashes rather than underscores. and a few are described differently.

We want to swap this around, so that we can start using the current names everywhere, making it easier to obsolete the legacy names when we remove the legacy probes in future. This will also help to avoid adding more references to the legacy names in the code.

Whilst here, I want to improve the TypeScript information around this - by switching to a frozen object, TypeScript will see it as not changing, and allow us to check against the keys or values of it.

Overall this should help with adding more types in future, and ensuring that we have more checking in place to avoid mistakes of submitting the wrong SAP source.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 2017131 - Include SearchUITestUtils.sys.mjs in TypeScript checking for browser/components/search. r?#search-reviewers!

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 2017131 - Rework BrowserSearchTelemetry.KNOWN_SOURCES to be a frozen object rather than a Map. r?#search-reviewers!

This helps with TypeScript support, since now TypeScript will know we do not intend to change the object,
and hence we can check for valid keys.

Comment 3

[Mark Banner (:standard8)]

Attachment: Bug 2017131 - Reverse the key/values of BrowserSearchTelemetry.KNOWN_SEARCH_SOURCES. r?#search-reviewers!

This changes it so that the deprecated sources (with '-') are now the values, and the sources that we use for newer
probes (with '_') are the keys. This will make it simpler for when we remove the deprecated telemetry.

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 2017131 - General update of TypeScript types. r?#frontend-codestyle-reviewers!

Comment 5

[Mark Banner (:standard8)]

Attachment: Bug 2017131 - Include SearchSERPTelemetry.sys.mjs in TypeScript checks. r?#search-reviewers!

Comment 6

[Mark Banner (:standard8)]

Attachment: Bug 2017131 - Add some more type information for SAP source in SearchSERPTelemetry.sys.mjs. r?#search-reviewers!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/29c4459f81bd
https://hg.mozilla.org/integration/autoland/rev/df34b57cc955
Include SearchUITestUtils.sys.mjs in TypeScript checking for browser/components/search. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/b0a75397bd02
https://hg.mozilla.org/integration/autoland/rev/97939a50edef
Rework BrowserSearchTelemetry.KNOWN_SOURCES to be a frozen object rather than a Map. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/7caaf3b24a4b
https://hg.mozilla.org/integration/autoland/rev/80d19e31f0c9
Reverse the key/values of BrowserSearchTelemetry.KNOWN_SEARCH_SOURCES. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/6d707c33b555
https://hg.mozilla.org/integration/autoland/rev/4a36531c6ac2
General update of TypeScript types. r=frontend-codestyle-reviewers,mossop
https://github.com/mozilla-firefox/firefox/commit/5b413306e068
https://hg.mozilla.org/integration/autoland/rev/fb8f27156fb9
Include SearchSERPTelemetry.sys.mjs in TypeScript checks. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/f1a24904e4c2
https://hg.mozilla.org/integration/autoland/rev/610be344092c
Add some more type information for SAP source in SearchSERPTelemetry.sys.mjs. r=search-reviewers,scunnane

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/532092fd8542
https://hg.mozilla.org/integration/autoland/rev/66f879e464e6
Revert "Bug 2017131 - Add some more type information for SAP source in SearchSERPTelemetry.sys.mjs. r=search-reviewers,scunnane" for causing documentation failures

Comment 9

[Sandor Molnar[:smolnar]]

Backed out for causing documentation failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/66f879e464e6a83ee2e67b62cfe3cedbb757b94e

Push with failures

Failure log -> sphinx.errors.ExtensionError: Handler <function analyze at 0x7fdf5b4d00e0> for event 'builder-inited' threw an exception (exception: Your code contains multiple documented objects at each of these paths:

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3d2cc9338a60
https://hg.mozilla.org/integration/autoland/rev/20eff6081322
Include SearchUITestUtils.sys.mjs in TypeScript checking for browser/components/search. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/7210c5064099
https://hg.mozilla.org/integration/autoland/rev/efb072447323
Rework BrowserSearchTelemetry.KNOWN_SOURCES to be a frozen object rather than a Map. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/b21a2fe063c4
https://hg.mozilla.org/integration/autoland/rev/bedefc0bdab1
Reverse the key/values of BrowserSearchTelemetry.KNOWN_SEARCH_SOURCES. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/6780b0f99e44
https://hg.mozilla.org/integration/autoland/rev/c44bcc177787
General update of TypeScript types. r=frontend-codestyle-reviewers,mossop
https://github.com/mozilla-firefox/firefox/commit/eb7cd0fa0725
https://hg.mozilla.org/integration/autoland/rev/5ef6c992bacd
Include SearchSERPTelemetry.sys.mjs in TypeScript checks. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/76376d80dcd0
https://hg.mozilla.org/integration/autoland/rev/d57620098852
Add some more type information for SAP source in SearchSERPTelemetry.sys.mjs. r=search-reviewers,scunnane

Comment 11

[Sandor Molnar[:smolnar]]

(In reply to Sandor Molnar[:smolnar] from comment #9)

Backed out for causing documentation failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/66f879e464e6a83ee2e67b62cfe3cedbb757b94e

Push with failures

Failure log -> sphinx.errors.ExtensionError: Handler <function analyze at 0x7fdf5b4d00e0> for event 'builder-inited' threw an exception (exception: Your code contains multiple documented objects at each of these paths:

relanded because some bc glean telemetry failures appeared that didn't pinpoint to anything.
hopefully it's from my revert

LE. it's not from this bug

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f7fc93f44d29
https://hg.mozilla.org/integration/autoland/rev/3dd3959ec9d1
Revert "Bug 2017131 - Add some more type information for SAP source in SearchSERPTelemetry.sys.mjs. r=search-reviewers,scunnane" for causing documentation failures

Comment 13

[Cristian Tuns]

(In reply to Sandor Molnar[:smolnar] from comment #9)

Backed out for causing documentation failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/66f879e464e6a83ee2e67b62cfe3cedbb757b94e

Push with failures

Failure log -> sphinx.errors.ExtensionError: Handler <function analyze at 0x7fdf5b4d00e0> for event 'builder-inited' threw an exception (exception: Your code contains multiple documented objects at each of these paths:

Backing out again, we are still having the documentation failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/3dd3959ec9d13bc716782da0944a07d595270d93

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0fce12de99c5
https://hg.mozilla.org/mozilla-central/rev/f9082c931991
Revert "Bug 2017131 - Add some more type information for SAP source in SearchSERPTelemetry.sys.mjs. r=search-reviewers,scunnane" for causing documentation failures

Comment 15

[Cristian Tuns]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/f9082c9319910b607e1b028c49b3bd353ab56f5a

Comment 16

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/20eff6081322
https://hg.mozilla.org/mozilla-central/rev/efb072447323
https://hg.mozilla.org/mozilla-central/rev/bedefc0bdab1
https://hg.mozilla.org/mozilla-central/rev/c44bcc177787
https://hg.mozilla.org/mozilla-central/rev/5ef6c992bacd
https://hg.mozilla.org/mozilla-central/rev/d57620098852

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Reopened for the backout in comment 15.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/85dd0d6ba068
https://hg.mozilla.org/integration/autoland/rev/6fb9e4d3b598
Include SearchUITestUtils.sys.mjs in TypeScript checking for browser/components/search. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/090f701092cf
https://hg.mozilla.org/integration/autoland/rev/9dd705656fe8
Rework BrowserSearchTelemetry.KNOWN_SOURCES to be a frozen object rather than a Map. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/d576832bce9f
https://hg.mozilla.org/integration/autoland/rev/cd4cc40abf78
Reverse the key/values of BrowserSearchTelemetry.KNOWN_SEARCH_SOURCES. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/0cca3109dd3e
https://hg.mozilla.org/integration/autoland/rev/9e0313a6bb68
General update of TypeScript types. r=frontend-codestyle-reviewers,mossop
https://github.com/mozilla-firefox/firefox/commit/7787d51cee24
https://hg.mozilla.org/integration/autoland/rev/bc0fb266ac1a
Include SearchSERPTelemetry.sys.mjs in TypeScript checks. r=search-reviewers,scunnane
https://github.com/mozilla-firefox/firefox/commit/646130289195
https://hg.mozilla.org/integration/autoland/rev/6fcf2a322564
Add some more type information for SAP source in SearchSERPTelemetry.sys.mjs. r=search-reviewers,scunnane