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

fix(load): updated queue utils #12

Merged
merged 3 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
11 changes: 11 additions & 0 deletions src/lib/browser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Checks if the current environment is a browser.
*
* This function verifies whether the code is being executed in a browser environment.
* It checks for the presence of the `window` object, which is unique to browsers.
*
* @returns {boolean} - Returns `true` if the current environment is a browser, otherwise `false`.
*/
export const isBrowser = () => {
return typeof window !== 'undefined' && typeof window.document !== 'undefined';
};
154 changes: 129 additions & 25 deletions src/lib/extension-load-queue.ts
Original file line number Diff line number Diff line change
@@ -1,63 +1,149 @@
/**
* This file provides utilities for managing and processing load queues in a browser environment.
*
* It includes:
* - A system for creating and managing load queues identified by unique symbols.
* - A script store that holds callbacks associated with controllers.
* - A React hook for using and managing controllers within a script store.
*
* Example usage:
*
* const MY_QUEUE_SYMBOL = Symbol.for('my-queue');
* const MY_STORE_SYMBOL = Symbol.for('my-controllers-store');
*
* const myStore = getScriptStore(MY_STORE_SYMBOL);
*
* createLoadQueue({
* store: myStore,
* createController: () => new MyController(),
* queueKey: MY_QUEUE_SYMBOL,
* });
*
* const controller = useController(myStore);
*/

import {useEffect, useState} from 'react';

import {isBrowser} from './browser';

export type ControllerLoadedCallback<T> = (controller: T) => void;

export const QUEUE_SYMBOL = Symbol.for('queue');
export const QUEUES_SYMBOL = Symbol.for('extension-load-queues');
export const SINGLE_QUEUE_SYMBOL = Symbol.for('single-queue');

export type ScriptStore<T> = ControllerLoadedCallback<T>[];

/**
* Interface for creating a load queue.
*/
export interface CreateLoadQueueArgs<T> {
// The store where callbacks are saved
store: ScriptStore<T>;

// Function that creates a controller
createController: () => T;

// Flag to check if the queue is already created
isQueueCreated?: boolean;

// Callback to handle queue creation

Choose a reason for hiding this comment

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

Interface members should be annotated with JSDoc-styled comments as well /** */

onQueueCreated?: (created: boolean) => void;
}

// TODO: this function is a very frequently used utility,
// so we should place it in a separate top-level file and document it.
export const isBrowser = () => typeof window !== 'undefined' && typeof document !== 'undefined';
// Symbol key for identifying the queue
queueKey?: symbol;
}

// TODO: window casts are weird — fix encounter any issues with declaration merging
export const getScriptStore = <T>(key: symbol): ScriptStore<T> => {
/**
* Retrieves or initializes a script store for the given symbol key.
*
* @param {symbol} storeKey - The key associated with the script store.
* @returns {ScriptStore<T>} The script store associated with the given key.
*/
export const getScriptStore = <T>(storeKey: symbol): ScriptStore<T> => {
if (isBrowser()) {
(window as Window)[key] = (window as Window)[key] || [];
return (window as Window)[key];
// Initialize the store if it does not exist or check that it is an array

Choose a reason for hiding this comment

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

In my opinion, this commenting style is too verbose. JSDoc is fine, but I don't think we really need to comment on every other statement.

if (!window[storeKey]) {
window[storeKey] = [];
} else if (!Array.isArray(window[storeKey])) {
throw new Error(`Expected window[${String(storeKey)}] to be an array`);
}

return window[storeKey] as ScriptStore<T>;
} else {
throw new Error('This functionality should be employed on the client-side.');
}
};

export const getQueueStore = () => {
/**
* Ensures that the queues are initialized as an object in the global window.
*
* @returns {void}
*/
const ensureQueuesSymbolInitialized = (): void => {
if (isBrowser()) {
(window as Window)[QUEUE_SYMBOL] = (window as Window)[QUEUE_SYMBOL] || false;
return (window as Window)[QUEUE_SYMBOL];
if (typeof window[QUEUES_SYMBOL] !== 'object' || window[QUEUES_SYMBOL] === null) {
window[QUEUES_SYMBOL] = {};
}
} else {
throw new Error('This functionality should be employed on the client-side.');

Choose a reason for hiding this comment

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

I don't think this error message is self-descriptive enough. The message should mention that it comes from a load queue. Maybe something along the lines of Cannot initialize QueueStore in a non-browser environment?

}
};

return null;
/**
* Retrieves the status of whether the queue associated with the queueKey is created.
*
* @param {symbol} queueKey - The key associated with the queue.
* @returns {boolean} Whether the queue is created.
*/
export const getQueueStore = (queueKey: symbol): boolean => {
ensureQueuesSymbolInitialized();
return window[QUEUES_SYMBOL]?.[queueKey] || false;
};

export const handleQueueCreated = (created: boolean) => {
(window as Window)[QUEUE_SYMBOL] = created;
/**
* Returns a function to mark the queue as created for a given queueKey.
*
* @param {symbol} queueKey - The key associated with the queue.
* @returns {function(boolean): void} A function that takes a boolean `created`
* and marks the queue as created.
*/
export const createHandleQueueCreated = (queueKey: symbol): ((created: boolean) => void) => {
ensureQueuesSymbolInitialized();
return (created: boolean) => {
window[QUEUES_SYMBOL][queueKey] = created;
};
};

/**
* Creates and manages a load queue.
*
* @param {CreateLoadQueueArgs<T>} args - The arguments required to create a load queue.
* @returns {void}
*/
export const createLoadQueue = <T>({
store,
createController,
isQueueCreated = getQueueStore(),
onQueueCreated = handleQueueCreated,
}: CreateLoadQueueArgs<T>) => {
queueKey = SINGLE_QUEUE_SYMBOL,
isQueueCreated = getQueueStore(queueKey),
onQueueCreated = createHandleQueueCreated(queueKey),

Choose a reason for hiding this comment

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

I can't say I like the fact we can pass in custom onQueueCreated, which would effectively break the whole system — the default handler does a very important job of tracking which queues were initialized. So my guess is we don't want to expose this option to the end user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not want to associate the createLoadQueue with the window runtime, assuming that the queue logic could generally work with any data source (store). I suggested leaving the prop onQueueCreated.

However, I agree that createHandleQueueCreated function could not be exported.

}: CreateLoadQueueArgs<T>): void => {
if (!store || isQueueCreated) {
return;
}

// Mark the queue as created
onQueueCreated(true);

const controller = createController();

const queue = store.splice(0, store.length);

// Override the store.push method to add callbacks to the queue and start processing
store.push = function (...args) {
args.forEach((callback) => {
queue.push(callback);

// Start processing the queue
unqueue();
});

Expand All @@ -66,12 +152,14 @@ export const createLoadQueue = <T>({

let processing = false;

// Function to start processing the next callback in the queue
function unqueue() {
if (!processing) {
next();
}
}

// Function to process callbacks in the queue one by one
async function next(): Promise<void> {
processing = true;

Expand All @@ -81,27 +169,44 @@ export const createLoadQueue = <T>({
return next();
}

// Mark the processing as complete
processing = false;
}

// Start the queue processing
unqueue();
};

const noop = () => {};

export function useController<T>(store: ScriptStore<T>) {
/**
* A no-operation function used as a default cleanup function.
*
* @returns {void}
*/
const noop = (): void => {};

/**
* React hook to manage and use a controller with a script store.
*
* @param {ScriptStore<T>} store - The store where the controller is managed.
* @returns {T | null} The current controller or null if not available.
*/
export function useController<T>(store: ScriptStore<T>): T | null {
const [controller, setController] = useState<T | null>(null);

useEffect(() => {
if (store) {
store.push(setController);
store.push(setController); // Add setController to the store

return () => {
const index = store.indexOf(setController);
if (index > -1) {
// Remove setController when unmounting
store.splice(index, 1);
}
};
} else {
// eslint-disable-next-line no-console
console.warn('Store is not provided to useController'); // Replace console.warn with a logging function if necessary

Choose a reason for hiding this comment

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

I'd say just throwing makes more sense here, maybe even removing the if (store) check altogether, since store cannot be nullable judging from the argument types

}

return noop;
Expand All @@ -112,8 +217,7 @@ export function useController<T>(store: ScriptStore<T>) {

declare global {
interface Window {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: symbol]: any; // TODO: sub-optimal, fix any
QUEUE_SYMBOL: boolean;
[key: symbol]: ScriptStore<unknown>;
[QUEUES_SYMBOL]: Record<symbol, boolean>;
}
}
6 changes: 3 additions & 3 deletions src/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@ import {
ControllerLoadedCallback,
CreateLoadQueueArgs,
ScriptStore,
createHandleQueueCreated,
createLoadQueue,
getQueueStore,
getScriptStore,
handleQueueCreated,
isBrowser,
useController,
} from './extension-load-queue';
import {isBrowser} from './browser';

export {
AttrsParser,
createLoadQueue,
getQueueStore,
getScriptStore,
handleQueueCreated,
createHandleQueueCreated,

Choose a reason for hiding this comment

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

Do we really need to export this? This seems like an internal handler.

isBrowser,
useController,
};
Expand Down
Loading