Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor getReadTheDocsConfig to use Promise #286

Merged
merged 4 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions dist/readthedocs-addons.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/readthedocs-addons.js.map

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
document.addEventListener(
"readthedocs-addons-data-ready",
function (event) {
console.debug(event.detail);
}
const data = event.detail.data();
console.debug(`Project slug using CustomEvent: '${data.projects.current.slug}'`);
}
);
</script>
<meta name="readthedocs-resolver-filename" content="/index.html" />
Expand Down
44 changes: 44 additions & 0 deletions src/events.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { getMetadataAddonsAPIVersion } from "./readthedocs-config";
import { ADDONS_API_VERSION } from "./utils";

export const EVENT_READTHEDOCS_SEARCH_SHOW = "readthedocs-search-show";
export const EVENT_READTHEDOCS_SEARCH_HIDE = "readthedocs-search-hide";
export const EVENT_READTHEDOCS_DOCDIFF_ADDED_REMOVED_SHOW =
Expand All @@ -7,3 +10,44 @@ export const EVENT_READTHEDOCS_FLYOUT_SHOW = "readthedocs-flyout-show";
export const EVENT_READTHEDOCS_FLYOUT_HIDE = "readthedocs-flyout-hide";
export const EVENT_READTHEDOCS_ADDONS_DATA_READY =
"readthedocs-addons-data-ready";

/**
* Object to pass to user subscribing to `EVENT_READTHEDOCS_ADDONS_DATA_READY`.
*
* This object allows us to have a better communication with the user.
* Instead of passing the raw data, we pass this object and enforce them
* to use it in an expected way:
*
* document.addEventListener(
* "readthedocs-addons-data-ready",
* function (event) {
* const data = event.detail.data();
* }
* );
*
* Note that we perform some checks/validations when `.data()` is called,
* to make sure the user is using the pattern in the expected way.
* Otherwise, we throw an exception.
*/
export class ReadTheDocsEventData {
constructor(data) {
this._initialized = false;
this._data = data;
}

initialize() {
const metadataAddonsAPIVersion = getMetadataAddonsAPIVersion();
if (metadataAddonsAPIVersion === undefined) {
throw `Subscribing to '${EVENT_READTHEDOCS_ADDONS_DATA_READY}' requires defining the '<meta name="readthedocs-addons-api-version" content="${ADDONS_API_VERSION}" />' tag in the HTML.`;
}

this._initialized = true;
}

data() {
if (!this._initialized) {
this.initialize();
}
return this._data;
}
}
4 changes: 2 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ export function setup() {
if (addon.isEnabled(config)) {
promises.push(
new Promise((resolve) => {
resolve(new addon(config));
return resolve(new addon(config));
}),
);
}
}
return Promise.all(promises);
})
.then(() => {
resolve();
return resolve();
})
.catch((err) => {
console.error(err);
Expand Down
3 changes: 1 addition & 2 deletions src/notification.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ export class NotificationElement extends LitElement {
//
// This does not cover all the cases where this notification could be useful,
// but users with different needs should be able to implement their own custom logic.
const versions = this.config.addons.non_latest_version_warning.versions;
const stable_index = versions.indexOf("stable");
const stable_index = this.config.versions.active.indexOf("stable");
const current_version = this.config.versions.current;
const current_project = this.config.projects.current;

Expand Down
149 changes: 89 additions & 60 deletions src/readthedocs-config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { default as fetch } from "unfetch";
import { EVENT_READTHEDOCS_ADDONS_DATA_READY } from "./events";
import {
EVENT_READTHEDOCS_ADDONS_DATA_READY,
ReadTheDocsEventData,
} from "./events";
import {
CLIENT_VERSION,
IS_TESTING,
Expand All @@ -11,21 +14,23 @@ import {
* Get the Read the Docs API version supported by user's integrations.
*
*/
function _getMetadataAddonsAPIVersion() {
export function getMetadataAddonsAPIVersion() {
const meta = document.querySelector(
"meta[name=readthedocs-addons-api-version]",
);
if (meta !== undefined) {
if (meta !== null) {
return meta.getAttribute("content");
}
return undefined;
}

/**
* Load Read the Docs configuration from API endpoint.
* Get the Addons API endpoint URL to hit.
*
* It uses META HTML tags to get project/version slugs and `sendUrlParam` to
* decide whether or not sending `url=`.
*/
export function getReadTheDocsConfig(sendUrlParam) {
function _getApiUrl(sendUrlParam, apiVersion) {
const metaProject = document.querySelector(
"meta[name='readthedocs-project-slug']",
);
Expand All @@ -37,7 +42,7 @@ export function getReadTheDocsConfig(sendUrlParam) {
let versionSlug;
let params = {
"client-version": CLIENT_VERSION,
"api-version": ADDONS_API_VERSION,
"api-version": apiVersion,
};

if (sendUrlParam) {
Expand All @@ -59,63 +64,87 @@ export function getReadTheDocsConfig(sendUrlParam) {
url = "/_/readthedocs-addons.json";
}

return fetch(url, {
method: "GET",
})
.then((response) => {
if (!response.ok) {
throw "Error parsing configuration data";
}
return response.json();
})
.then((data) => {
// We force the user to define the `<meta>` tag to be able to use Read the Docs data directly.
// This is to keep forward/backward compatibility without breaking integrations.
const metadataAddonsAPIVersion = _getMetadataAddonsAPIVersion();
if (metadataAddonsAPIVersion !== undefined) {
if (metadataAddonsAPIVersion !== data.api_version) {
// When the API scheme version returned doesn't match the one defined via `<meta>` tag by the user,
// we perform another request to get the Read the Docs response in the structure
// that's supported by the user and dispatch a custom event letting them know
// this data is ready to be consumed under `event.detail`.

url =
ADDONS_API_ENDPOINT +
new URLSearchParams({
url: window.location.href,
"client-version": CLIENT_VERSION,
"api-version": metadataAddonsAPIVersion,
});

fetch(url, {
method: "GET",
})
.then((response) => {
if (!response.ok) {
throw "Error parsing configuration data";
}
return response.json();
})
.then((data) => {
dispatchEvent(
EVENT_READTHEDOCS_ADDONS_DATA_READY,
document,
data,
);
})
.catch((error) => {
console.error(error);
});
} else {
dispatchEvent(EVENT_READTHEDOCS_ADDONS_DATA_READY, document, data);
return url;
}

function getReadTheDocsUserConfig(sendUrlParam) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking this out is a good idea 👍 Often, splitting up nested async/promises makes the code easier to follow

// Create a Promise here to handle the user request in a different async task.
// This allows us to start executing our integration independently from the user one.
return new Promise((resolve, reject) => {
// Note we force the user to define the `<meta>` tag to be able to use Read the Docs data directly.
// This is to keep forward/backward compatibility without breaking integrations.
const metadataAddonsAPIVersion = getMetadataAddonsAPIVersion();

if (
metadataAddonsAPIVersion !== undefined &&
metadataAddonsAPIVersion !== ADDONS_API_VERSION
) {
// When the addons API version doesn't match the one defined via `<meta>` tag by the user,
// we perform another request to get the Read the Docs response in the structure
// that's supported by the user and dispatch a custom event letting them know
// this data is ready to be consumed under `event.detail.data()`.
const userApiUrl = _getApiUrl(sendUrlParam, metadataAddonsAPIVersion);

// TODO: revert this change and use the correct URL here
const url = "/_/readthedocs-addons.json";
fetch(url, {
method: "GET",
}).then((response) => {
if (!response.ok) {
return reject(
"Error hitting addons API endpoint for user api-version",
);
}
}
// Return the data in the API version requested.
return resolve(response.json());
});
}

// If the API versions match, we return `undefined`.
return resolve(undefined);
}).catch((error) => {
console.error(error);
});
}

return data;
/**
* Load Read the Docs configuration from API endpoint.
*
*/
export function getReadTheDocsConfig(sendUrlParam) {
return new Promise((resolve, reject) => {
let dataUser;
const defaultApiUrl = _getApiUrl(sendUrlParam, ADDONS_API_VERSION);

fetch(defaultApiUrl, {
method: "GET",
})
.catch((error) => {
console.error(error);
});
.then((response) => {
if (!response.ok) {
return reject("Error hitting addons API endpoint");
}
return response.json();
})
.then((data) => {
// Trigger a new task here to hit the API again in case the version
// request missmatchs the one the user expects.
getReadTheDocsUserConfig(sendUrlParam).then((dataUser) => {
// Expose `dataUser` if available or the `data` already requested.
const dataEvent = dataUser !== undefined ? dataUser : data;

// Trigger the addons data ready CustomEvent to with the data the user is expecting.
return dispatchEvent(
EVENT_READTHEDOCS_ADDONS_DATA_READY,
document,
new ReadTheDocsEventData(dataEvent),
);
});

return resolve(data);
});
}).catch((error) => {
console.error(error);
});
}

function dispatchEvent(eventName, element, data) {
Expand Down
2 changes: 1 addition & 1 deletion tests/index.test.html
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
describe("Main library", () => {
it("hits Read the Docs addons API", async () => {
const matchUrl = new RegExp(`^${ADDONS_API_ENDPOINT}`, "g");
server.respondWith("GET", matchUrl, [200, {}, "{}"]);
server.respondWith("GET", matchUrl, [200, {}, '{"testing": true}']);

// Our .setup() function returns a Promise here and we want to wait for it.
await readthedocs.setup();
Expand Down