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

Alternative api #48

Merged
merged 34 commits into from
May 11, 2024
Merged

Alternative api #48

merged 34 commits into from
May 11, 2024

Conversation

TGlide
Copy link
Member

@TGlide TGlide commented May 5, 2024

Exploring some alternative APIs for our existing utilities

@TGlide TGlide self-assigned this May 5, 2024
Copy link

changeset-bot bot commented May 5, 2024

🦋 Changeset detected

Latest commit: fd7a511

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
runed Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented May 5, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
runed ✅ Ready (View Log) Visit Preview fd7a511

@abdel-17 abdel-17 mentioned this pull request May 7, 2024
@abdel-17
Copy link
Collaborator

abdel-17 commented May 10, 2024

I suggest we keep the external API using functions and keep classes an implementation detail. Classes have a predetermined shape and cannot be overloaded, so something like #50 would have to be separated to ReadableStore and WritableStore.

An alternative is to have a static method attached to the class to handle overloads.

abstract class Store<T> {
  abstract value: T;

  static from<T>(readable: Readable<T>): Readonly<Store<T>>

  static from<T>(writable: Writable<T>): Store<T>

  static from<T>(store: Readable<T> | Writable<T>): Store<T> {
    if (isWritable(store)) {
      return new WritableStore(store);
    }
    return new ReadableStore(store);
  }
}

class ReadableStore<T> extends Store<T> { ... }

class WritableStore<T> extends Store<T> { ... }

@github-actions github-actions bot requested a deployment to Preview May 11, 2024 23:19 Abandoned
@TGlide TGlide merged commit db9ee97 into main May 11, 2024
4 checks passed
@niemyjski
Copy link

I'm curious why box was removed? Just trying to learn more

@huntabyte huntabyte deleted the alt-api branch June 30, 2024 19:26
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.

4 participants