Scope "shippeable" v1 of this js client

[humitos]

What's is required to ship a v1¹ of this client and expose to users using build.commands, which currently have 0 integrations with Read the Docs?

After some talks, we started to treat each feature as a separated feature that can be deployed without deploying the "whole flyout". In fact, we decided we want to deploy other features first without a flyout at all.

Functionality

There are some features that require zero configuration from the user at this point (they can always be expanded to be customized, of course, but we are not there yet. See #13 for that)

Some of these features are:

Feature	Description	Data required	Triggered by
Analytics	Hits an API endpoint to show analytics in the dashboard	Project (slug, language, programming language), Version (slug), Global analytics code	Automatic (always)
External version warning	Shows a warning banner if the version is external	Version (type), Domain (dashboard), Project (slug, repository_url), Build (id)	Automatic (when the version is external)
Search as you type	Presents a modal to search using our own backend	Project (slug)	Manual (by hitting /)

Backend

Where the data required will come from?

• I'm currently using the new endpoint /_/readthedocs-config/?url= with a data structured that was not thought completely and returns a bunch of duplicated data for now
• We could polish it a little to return the real objects and define it in an extendable way so we can include more data in the future
• The actual structure proposes an idea for this. In particular using:
  • Top level keys for "generic" data, like project, version, domain objects that are not tied to a specific feature
  • Feature-specific data under features.<feature>, like features.external_version_warning, features.analytics, etc

CF worker that doesn't randomly 500

We are not sure why, but our script running in CF sometimes it break for some reason and returns an error 1/2 the times we hit it. We will need to debug this.

Return X-RTD-Hosting-Integrations on versions with build.commands

We will need to modify the Proxito middleware to check if the version is using build.commands and inject the header in that case so CF includes our client.

Frontend

It would be good if I can get some review on the current HTML structure of these features. I can make the required adjustments to it. However, I would avoid getting too deep into creating the perfect structure since we are not going to officially allow people to style/customize/change them for now.

Footnotes

1. we are calling internally v1. This does not mean it will be a v1 for our customers/users. It will be a beta feature for now until we establish some standards and we feel comfortable with it to promote it and write documentation about it. ↩

[humitos]

We chatted about this in Slack and we agreed on: "deploy a v0.5 with just the analytics, and do following deploy with the PR warning banner"

The following are the list of pending tasks for the v0.5 deploy:

• review the Analytics code (@agjohnson maybe?)
• polish CF worker js code (@humitos)
• update Proxito to include the HTTP header for build.commands (@humitos)

There was one thing we missed from our conversation since we detoured talking about #17 😄. @ericholscher @agjohnson are we OK using the current API structure for now? I guess yes, but we didn't touch on that.

[agjohnson]

are we OK using the current API structure for now

That's hard to say. It looks sufficient for now. But I also see how we'll change this in the future.

I think we're still at a place that anything really goes, so probably don't need to do anything else here yet. But we should have some milestone in mind for firming up the API structure to be a bit more future proof -- and avoid backporting as much as possible.

review the Analytics code

I can jump in here and on the other frontend reviews, perhaps my next week at this point. I do want to start considering timeframes more seriously here though, as my main priority is still the dashbaord and I don't want to pull away from that priority much. Having some help on the dashboard would also give me some room to put attention into other projects.

[humitos]

@agjohnson

That's hard to say. It looks sufficient for now. But I also see how we'll change this in the future.

Yeah, I have the same feeling. However, for now, if we are always returning the latest client (without dealing with the versioning yet) we can deploy both changes at the same time: API + js client, and there should be a pretty small timeframe window where some documentation pages break.

In any case, I will review the current JSON returned, and may propose some initial tweaks thinking on what we already talk (e.g. use the APIv3 serializers -- see https://github.com/readthedocs/readthedocs-ops/issues/1323), so we can add fields keeping the same structure without introducing breaking changes.

I will open a PR with these changes in the API response so we can talk a little about it as well.

[humitos]

I created some PRs to achieve the initial goals here. I think we are ready to get review and move forward after that.

[humitos]

The plan is going a little slower than we thought because we started adding Lit on the external version warning and that required some extra time from Anthony to help me there with the pattern.

So, I think we are not yet ready to deploy that. I need to explore and play a little more with those changes to feel more comfortable before deploying that. However, I still think we should deploy something. I'd appreciate if I can get a review from @agjohnson in https://github.com/readthedocs/readthedocs-client/blob/main/src/analytics.js which does not require any UI/UX and it will be useful to use it as an initial test for the new architecture (CF, .js file, API response, etc).

We detoured a little from the original plan, but I think it's for good reasons. However, I do want to deploy something if we can. We are just a few reviews away from that 😉

[agjohnson]

Agreed, I think adding Lit is a digestible step here and does help us immediately to scope down CSS concerns and provide something a little customizable to start. It doesn't feel too far off even with this addition.

[ericholscher]

I'm 👍 on shipping just the analytics as a v1 though. I agree that enabling Cloudflare workers more widely, and injecting the client with no visible UX, is still gonna be a pretty big change. I'm a little worried about blowing through our CF Worker quotas, so we might need a bit more dev and research there, so that's gonna be a valuable data point even in that v1 deployment.

[humitos]

@agjohnson I updated the https://github.com/readthedocs/readthedocs-client/blob/95e07b46cb764d31c76f2fc978602001d9d6b828/src/analytics.js file to extend AddonBase class and follow that pattern. I think we are ready to comment all the other features that are not migrated to that pattern and deploy it. That deploy will include only the "Notification for pull requests" and "Read the Docs analytics"

[agjohnson]

You brought up a good point in my PR and I opened another at #48

I updated the analytics in that PR as well, but otherwise you change looks great!

[humitos]

I'm closing this issue. We have everything in place already. There are other issues/prs that are waiting for review/merge; but we can continue the conversation over specific details there.

All the plans from this issue are already done and there is nothing else to discuss, I guess. Feel free to reopen, tho. Thanks you all for chiming in and help with all of this 💪🏼