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.

Sign up for GitHub

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

Add validation and some small cleanup #19

Merged
merged 2 commits into from
Sep 20, 2024
Merged

Add validation and some small cleanup #19

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
27648a1
Add validation and some cleanup
peternandersson Sep 20, 2024
ea21fc4
use helper function
peternandersson Sep 20, 2024
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
46 changes: 23 additions & 23 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,36 +434,36 @@ interface PluginInstance<T> {
/**
* Gets a static image of a workbook variable
*/
getVariable(id: string): WorkbookVariable;
getVariable(configId: string): WorkbookVariable;

/**
* Setter for workbook variable passed in
*/
setVariable(id: string, ...values: unknown[]): void;
setVariable(configId: string, ...values: unknown[]): void;

/**
* Getter for interaction selection state
*/
getInteraction(id: string): WorkbookSelection[];
getInteraction(configId: string): WorkbookSelection[];

/**
* Setter for interaction selection state
*/
setInteraction(
id: string,
configId: string,
elementId: string,
selection: WorkbookSelection[],
): void;

/**
* Triggers an action based on the provided action trigger ID
*/
triggerAction(id: string): void;
triggerAction(configId: string): void;

/**
* Registers an effect with the provided action effect ID
*/
registerEffect(id: string, effect: Function): void;
registerEffect(configId: string, effect: Function): void;

/**
* Overrider function for Config Ready state
Expand All @@ -474,7 +474,7 @@ interface PluginInstance<T> {
* Allows users to subscribe to changes in the passed in variable
*/
subscribeToWorkbookVariable(
id: string,
configId: string,
callback: (input: WorkbookVariable) => void,
): Unsubscriber;

Expand All @@ -483,7 +483,7 @@ interface PluginInstance<T> {
* Allows users to subscribe to changes in the passed in interaction ID
*/
subscribeToWorkbookInteraction(
id: string,
configId: string,
callback: (input: WorkbookSelection[]) => void,
): Unsubscriber;
};
Expand All @@ -492,21 +492,21 @@ interface PluginInstance<T> {
/**
* Getter for Column Data by parent sheet ID
*/
getElementColumns(id: string): Promise<WbElementColumns>;
getElementColumns(configId: string): Promise<WbElementColumns>;

/**
* Subscriber to changes in column data by ID
*/
subscribeToElementColumns(
id: string,
configId: string,
callback: (cols: WbElementColumns) => void,
): Unsubscriber;

/**
* Subscriber for the data within a given sheet
*/
subscribeToElementData(
id: string,
configId: string,
callback: (data: WbElementData) => void,
): Unsubscriber;
};
Expand Down Expand Up @@ -647,12 +647,12 @@ interface WorkbookElementColumns {
Provides the latest data values from corresponding sheet, up to 25000 values.

```ts
function useElementData(elementId: string): WorkbookElementData;
function useElementData(configId: string): WorkbookElementData;
```

Arguments

- `elementId : string` - A workbook element’s unique identifier.
- `configId : string` - A workbook element’s unique identifier from the plugin config.

Returns the row data from the specified element.

Expand All @@ -668,12 +668,12 @@ Provides the latest data values from the corresponding sheet (initially 25000),
callback for fetching more data in chunks of 25000 values.

```ts
function useElementData(elementId: string): [WorkbookElementData, () => void];
function useElementData(configId: string): [WorkbookElementData, () => void];
```

Arguments

- `elementId : string` - A workbook element’s unique identifier.
- `configId : string` - A workbook element’s unique identifier from the plugin config.

Returns the row data from the specified element, and a callback for fetching
more data.
Expand All @@ -690,13 +690,13 @@ Returns a given variable's value and a setter to update that variable

```ts
function useVariable(
variableId: string,
configId: string,
): [WorkbookVariable | undefined, (...values: unknown[]) => void];
```

Arguments

- `variableId : string` - The ID of the variable
- `configId : string` - The config ID corresponding to the workbook control variable

The returned setter function accepts 1 or more variable values expressed as an
array or multiple parameters
Expand All @@ -711,14 +711,14 @@ Returns a given interaction's selection state and a setter to update that intera

```ts
function useInteraction(
interactionId: string,
configId: string,
elementId: string,
): [WorkbookSelection | undefined, (value: WorkbookSelection[]) => void];
```

Arguments

- `interactionId : string` - The ID of the interaction
- `configId : string` - The config ID corresponding to the workbook interaction
- `elementId : string` - The ID of the element that this interaction is
associated with

Expand All @@ -730,7 +730,7 @@ function setVariableCallback(value: WorkbookSelection[]): void;

#### useActionTrigger()

- `configId : string` - The ID of the action trigger from the Plugin Config
- `configId : string` - The config ID corresponding to the action trigger

Returns a callback function to trigger one or more action effects for a given action trigger

Expand All @@ -742,7 +742,7 @@ function useActionTrigger(configId: string): () => void;

Arguments

- `configId : string` - The ID of the action trigger from the Plugin Config
- `configId : string` - The config ID corresponding to the action trigger

The function that can be called to asynchronously trigger the action

Expand All @@ -755,12 +755,12 @@ function triggerActionCallback(configId: string): void;
Registers and unregisters an action effect within the plugin

```ts
function useActionEffect(effectId: string, effect: () => void);
function useActionEffect(configId: string, effect: () => void);
```

Arguments

- `effectId : string` - The ID of the action effect
- `configId : string` - The config ID corresponding to the action effect
- `effect : Function` - The function to be called when the effect is triggered

#### useConfig()
Expand Down
95 changes: 63 additions & 32 deletions src/client/initialize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
PluginMessageResponse,
WorkbookSelection,
WorkbookVariable,
Unsubscriber,
} from '../types';

export function initialize<T = {}>(): PluginInstance<T> {
Expand Down Expand Up @@ -126,28 +125,48 @@ export function initialize<T = {}>(): PluginInstance<T> {
on('config', listener);
return () => off('config', listener);
},
getVariable(id: string): WorkbookVariable {
return subscribedWorkbookVars[id];
getVariable(configId: string) {
if (configId === undefined) {
peternandersson marked this conversation as resolved.
Show resolved Hide resolved
console.warn(`Invalid config variable: ${configId}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we should mark these as deprecation warnings and indicate that the ability to do this will be removed in the next major version?

Suggested change
console.warn(`Invalid config variable: ${configId}`);
console.warn(`Deprecation warning: variable ID must be defined. The ability to pass an undefined ID will be removed in the next major version`);

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 think an error like this might lead new users to believe that having a defined config id is currently valid, but part of the issue with throwing here in a future version, is that when their app is not embedded into slate, it will throw an error from the config not being defined. This could be acceptable behavior, but I personally like the softer warning when the config isn't defined.

That being said, I think a good expansion to this in the future is to potentially rework the config system so that we can validate that a config value corresponds to a config option of the right type instead of just ensuring it's not undefined.

}
return subscribedWorkbookVars[configId];
},
setVariable(id: string, ...values: unknown[]) {
void execPromise('wb:plugin:variable:set', id, ...values);
setVariable(configId: string, ...values: unknown[]) {
if (configId === undefined) {
console.warn(`Invalid config variable: ${configId}`);
}
void execPromise('wb:plugin:variable:set', configId, ...values);
},
getInteraction(id: string) {
return subscribedInteractions[id];
getInteraction(configId: string) {
if (configId === undefined) {
console.warn(`Invalid config interaction: ${configId}`);
}
return subscribedInteractions[configId];
},
setInteraction(
id: string,
configId: string,
elementId: string,
selection:
| string[]
| Array<Record<string, { type: string; val?: unknown }>>,
) {
void execPromise('wb:plugin:selection:set', id, elementId, selection);
void execPromise(
'wb:plugin:selection:set',
configId,
elementId,
selection,
);
},
triggerAction(configId: string) {
if (configId === undefined) {
console.warn(`Invalid config action trigger: ${configId}`);
}
void execPromise('wb:plugin:action-trigger:invoke', configId);
},
registerEffect(configId: string, effect: () => void) {
if (configId === undefined) {
console.warn(`Invalid config action effect: ${configId}`);
}
registeredEffects[configId] = effect;
return () => {
delete registeredEffects[configId];
Expand All @@ -159,24 +178,24 @@ export function initialize<T = {}>(): PluginInstance<T> {
setLoadingState(loadingState) {
void execPromise('wb:plugin:config:loading-state', loadingState);
},
subscribeToWorkbookVariable(
id: string,
callback: (input: WorkbookVariable) => void,
): Unsubscriber {
subscribeToWorkbookVariable(configId, callback) {
if (configId === undefined) {
console.warn(`Invalid config variable: ${configId}`);
}
const setValues = (values: Record<string, WorkbookVariable>) => {
callback(values[id]);
callback(values[configId]);
};
on('wb:plugin:variable:update', setValues);
return () => {
off('wb:plugin:variable:update', setValues);
};
},
subscribeToWorkbookInteraction(
id: string,
callback: (input: WorkbookSelection[]) => void,
): Unsubscriber {
subscribeToWorkbookInteraction(configId, callback) {
if (configId === undefined) {
console.warn(`Invalid config interaction: ${configId}`);
}
const setValues = (values: Record<string, WorkbookSelection[]>) => {
callback(values[id]);
callback(values[configId]);
};
on('wb:plugin:selection:update', setValues);
return () => {
Expand All @@ -185,31 +204,43 @@ export function initialize<T = {}>(): PluginInstance<T> {
},
},
elements: {
getElementColumns(id) {
return execPromise('wb:plugin:element:columns:get', id);
},
subscribeToElementColumns(id, callback) {
const eventName = `wb:plugin:element:${id}:columns`;
getElementColumns(configId) {
if (configId === undefined) {
console.warn(`Invalid config element: ${configId}`);
}
return execPromise('wb:plugin:element:columns:get', configId);
},
subscribeToElementColumns(configId, callback) {
if (configId === undefined) {
console.warn(`Invalid config element: ${configId}`);
}
const eventName = `wb:plugin:element:${configId}:columns`;
on(eventName, callback);
void execPromise('wb:plugin:element:subscribe:columns', id);
void execPromise('wb:plugin:element:subscribe:columns', configId);

return () => {
off(eventName, callback);
void execPromise('wb:plugin:element:unsubscribe:columns', id);
void execPromise('wb:plugin:element:unsubscribe:columns', configId);
};
},
subscribeToElementData(id, callback) {
const eventName = `wb:plugin:element:${id}:data`;
subscribeToElementData(configId, callback) {
if (configId === undefined) {
console.warn(`Invalid config element: ${configId}`);
}
const eventName = `wb:plugin:element:${configId}:data`;
on(eventName, callback);
void execPromise('wb:plugin:element:subscribe:data', id);
void execPromise('wb:plugin:element:subscribe:data', configId);

return () => {
off(eventName, callback);
void execPromise('wb:plugin:element:unsubscribe:data', id);
void execPromise('wb:plugin:element:unsubscribe:data', configId);
};
},
fetchMoreElementData(id) {
void execPromise('wb:plugin:element:fetch-more', id);
fetchMoreElementData(configId) {
if (configId === undefined) {
console.warn(`Invalid config element: ${configId}`);
}
void execPromise('wb:plugin:element:fetch-more', configId);
},
},
destroy() {
Expand Down
Loading