Suggest making HybridContentTelemetry-lib.js into an npm package

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P1
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: scolville, Assigned: Dexter)

Tracking

(Blocks 1 bug)

unspecified
mozilla62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(2 attachments)

Using Hybrid content telemetry [1] in content requires installing a lib file from here[2]. 

Since copying the file into will require manual checking for updates it would be better if this file could be packaged on NPM instead. This way standard tools can be used to inform the consumer of updates e.g. greenkeeper. This would also allow  a clearer understanding of what version of the file is in use through package versions.

[1] https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/hybrid-content.html
[2] https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/hybrid-content/HybridContentTelemetry-lib.js
Blocks: 1416718
Priority: -- → P2
Assignee: nobody → alessio.placitelli
Priority: P2 → P1
Comment on attachment 8981174 [details]
Bug 1460851 - Package the hybrid content telemetry lib in npm.

https://reviewboard.mozilla.org/r/247268/#review253350

Seems fine, but I'm no npm expert.
Attachment #8981174 - Flags: review?(chutten) → review+
Comment on attachment 8981175 [details]
Bug 1460851 - Update the HCT docs to mention the NPM dependency.

https://reviewboard.mozilla.org/r/247270/#review253352
Attachment #8981175 - Flags: review?(chutten) → review+
Comment on attachment 8981174 [details]
Bug 1460851 - Package the hybrid content telemetry lib in npm.

https://reviewboard.mozilla.org/r/247268/#review253354

Looks reasonable (disclaimer: I am also not really an npm/node expert), I would suggest a followup to update the docs here: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/hybrid-content.html#hybrid-content-telemetry

A few things to add/change:

1. You can recommend people use/install the package from npm, and give instructions for doing so
2. We'll implicitly have a cdn for the package via unpkg (though it might still make sense to encourage people to bundle the file directly)

::: toolkit/components/telemetry/hybrid-content/package.json:2
(Diff revision 1)
> +{
> +  "name": "hybridcontenttelemetry-mozilla",

Nit: I think `mozilla-hybrid-content-telemetry` is slightly easier to read.
Attachment #8981174 - Flags: review?(wlachance) → review+
Hey Stuart,

do you know about publishing NPM packages? If so, do you want to review this to make sure we're not missing anything important?
Flags: needinfo?(scolville)
Comment on attachment 8981174 [details]
Bug 1460851 - Package the hybrid content telemetry lib in npm.

https://reviewboard.mozilla.org/r/247268/#review253580
Attachment #8981174 - Flags: review?(jrediger) → review+
Comment on attachment 8981175 [details]
Bug 1460851 - Update the HCT docs to mention the NPM dependency.

https://reviewboard.mozilla.org/r/247270/#review253586

::: toolkit/components/telemetry/docs/collection/hybrid-content.rst:60
(Diff revision 1)
>  
>      Granted permissions do not disappear when a "go-faster" add-on is uninstalled but are cleared when the browser is closed. If permissions need to be cleaned without closing the browser, it must be done manually. Moreover, permissions are keyed by origin: ``http://mozilla.com`` and ``https://mozilla.com`` are different things.
>  
>  Including the library
>  ---------------------
> -To use hybrid content telemetry the relative content JS library needs to be included in the page. We don't have a CDN hosted version that can be readily included in the page. For this reason, each consumer will need to fetch the latest version of the library from `here <https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/hybrid-content/HybridContentTelemetry-lib.js>`_ and add it to the page repository. Then this file can be deployed along with the page.
> +To use hybrid content telemetry the relative content JS library needs to be included in the page. We don't have a CDN hosted version that can be readily included in the page. For this reason, each consumer will need to include the library by adding a dependency to the NPM package which lives `here <https://npmjs.com/package/hybridcontenttelemetry-mozilla>`_. Alternatively, the consumer can manually fetch the latest version of the library from `here <https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/hybrid-content/HybridContentTelemetry-lib.js>`_ and add it to the page repository. Then this file can be deployed along with the page.

"the relative library" sounds wrong. Should this have been "related library"?

::: toolkit/components/telemetry/docs/collection/hybrid-content.rst:60
(Diff revision 1)
>  
>      Granted permissions do not disappear when a "go-faster" add-on is uninstalled but are cleared when the browser is closed. If permissions need to be cleaned without closing the browser, it must be done manually. Moreover, permissions are keyed by origin: ``http://mozilla.com`` and ``https://mozilla.com`` are different things.
>  
>  Including the library
>  ---------------------
> -To use hybrid content telemetry the relative content JS library needs to be included in the page. We don't have a CDN hosted version that can be readily included in the page. For this reason, each consumer will need to fetch the latest version of the library from `here <https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/hybrid-content/HybridContentTelemetry-lib.js>`_ and add it to the page repository. Then this file can be deployed along with the page.
> +To use hybrid content telemetry the relative content JS library needs to be included in the page. We don't have a CDN hosted version that can be readily included in the page. For this reason, each consumer will need to include the library by adding a dependency to the NPM package which lives `here <https://npmjs.com/package/hybridcontenttelemetry-mozilla>`_. Alternatively, the consumer can manually fetch the latest version of the library from `here <https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/hybrid-content/HybridContentTelemetry-lib.js>`_ and add it to the page repository. Then this file can be deployed along with the page.

As mentioned in chat, something along the lines below would improve this:

* Add `mozilla-hybrid-content-telemetry` as a dependency to your project and require it in your code.
* Load it directly from the external unpkg CDN
* or manually fetch the latest version from the main repository

I'd suggest not using "here" as the link as it is non-descriptive, repetitive and a smaller-than-necessary click target, making this less accessible.
Attachment #8981175 - Flags: review?(jrediger) → review+
Comment on attachment 8981174 [details]
Bug 1460851 - Package the hybrid content telemetry lib in npm.

https://reviewboard.mozilla.org/r/247268/#review253628

As the usage requires upstream changes anyway (the permission) I think it's fine to only link to the upstream docu for this and we don't need a short API example inline.

See below nits for slightly improved formatting.

::: toolkit/components/telemetry/hybrid-content/readme.md:1
(Diff revision 2)
> +This package exports the content library for Mozilla's Hybrid Content Telemetry.

Add a header:

# Hybrid Content Telemetry

::: toolkit/components/telemetry/hybrid-content/readme.md:1
(Diff revision 2)
> +This package exports the content library for Mozilla's Hybrid Content Telemetry.

Maybe make this a bit more explanatory, using the doc's first paragraph maybe:


Hybrid content is web content that is loaded as part of Firefox, appears as part of Firefox to the user and is primarily intended to be used in Firefox.

Hybrid content telemetry allows Mozilla pages to check whether data collection is enabled and to submit Telemetry data.


(Maybe just the second one is enough?)

::: toolkit/components/telemetry/hybrid-content/readme.md:3
(Diff revision 2)
> +This package exports the content library for Mozilla's Hybrid Content Telemetry.
> +
> +For additional information, see the [official documentation](https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/hybrid-content.html).

Add a line about the License:

# License

MPL 2.0 License, copyright (c) 2018 ...
The NPM package is living here: https://www.npmjs.com/package/mozilla-hybrid-content-telemetry
Flags: needinfo?(scolville)
Pushed by alessio.placitelli@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/634f5d70401b
Package the hybrid content telemetry lib in npm. r=chutten,janerik,wlach
https://hg.mozilla.org/integration/autoland/rev/ca5eb1d22bf6
Update the HCT docs to mention the NPM dependency. r=chutten,janerik
https://hg.mozilla.org/mozilla-central/rev/634f5d70401b
https://hg.mozilla.org/mozilla-central/rev/ca5eb1d22bf6
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Depends on: 1467787
You need to log in before you can comment on or make changes to this bug.