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

Extensibility proof-of-concept #15

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Extensibility proof-of-concept #15

wants to merge 2 commits into from

Conversation

chancancode
Copy link
Owner

No description provided.

storage: Storage,
keys: string[],
): Promise<Array<string | null>> {
if (isEnabled(storage, 'batch')) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This allows type safe access to the 'batch' extension

export type Deserializer<T> = (serialized: SerializedObject) => T;

export default abstract class JSONStorage extends Service {
abstract storage: Storage;
Copy link
Owner Author

Choose a reason for hiding this comment

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

This requires the app to inject (and therefore compose over) the store, rather than extending it directly

new (scope: Scope): Storage & Ext;
};

const Extensions = new WeakMap<typeof Storage, Set<keyof Registry>>();
Copy link
Owner Author

Choose a reason for hiding this comment

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

On second thought, using a WeakMap here is not quite right; when a class implements/enables a feature, its subclass should also be considered an implementor, so something on the prototype inheritance chain works better. That's a small detail for production consideration, doesn't really affect the overall point this demo is trying to make

import JSONStorage, { type Deserializer } from 'ember-storage-json';
import Storage from './storage';

export class AppJSONStorage extends JSONStorage {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is only exported for type reasons, the default export is what's important

import Storage from './storage';

export class AppJSONStorage extends JSONStorage {
storage = service(this, Storage);
Copy link
Owner Author

Choose a reason for hiding this comment

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

Injects the store as is required – also pulls in the module dependency naturally


const found: T[] = [];

for (const [key, value] of this.storage.entries()) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Note that this works

import { enable } from 'ember-storage/extensions';
import { type StorageWithBatching } from 'ember-storage-batch';

export class AppStorage extends Storage implements StorageWithBatching {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This implements StorageWithBatching is nice but optional – even if you don't do this, you will still get the type error down below

this.#storage.removeItem(this.keyFor(key));
}

*entries(): Generator<[string, string], void, void> {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is accessed in the JSONStorage service as proof that these custom extensions can be used in the app just fine

// This is type safe!
// Even if you didn't write `implements StorageWithBatching` above,
// you will still get an error here.
enable(AppStorage, 'batch');
Copy link
Owner Author

Choose a reason for hiding this comment

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

This will error in TypeScript if AppStorage (the instance side) does not in fact implement the 'batch' interface

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant