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

Add models namespace and constructor on client #58

Merged
merged 5 commits into from
Oct 20, 2023

Conversation

zknill
Copy link
Collaborator

@zknill zknill commented Oct 19, 2023

Update Models to be called ModelsClient, the client gives access to models using:

modelsClient.models.get<T>({...})

The get function accepts the model name, channel name, and registration arguments ($sync and $merge functions) in order to build and register the model.

Update Models to be called ModelsClient, the client gives access to
models using:

```
modelsClient.models.get<T>({...})
```

The get function accepts the model name, channel name, and registration
arguments ($sync and $merge functions) in order to build and register
the model.
@zknill zknill requested a review from mschristensen October 19, 2023 10:50
README.md Show resolved Hide resolved
docs/concepts/usage.md Outdated Show resolved Hide resolved
@zknill zknill force-pushed the zak/COL-41-models-namespace branch from 6141ae5 to d8e3018 Compare October 19, 2023 14:16
src/ModelsClient.ts Outdated Show resolved Hide resolved
Require users to call the initial sync function themselves.
Remove the $register function, and move the method registrations into
the Model constructor.

Update the docs
src/ModelsClient.ts Outdated Show resolved Hide resolved
@zknill zknill requested a review from mschristensen October 20, 2023 12:12
Copy link
Collaborator

@mschristensen mschristensen left a comment

Choose a reason for hiding this comment

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

Looks good code wise but I'll leave it to you to double check the docs with the await

README.md Outdated
// register the functions we defined above
await model.$register({
// create a new model instance called 'post' by passing the sync and merge functions
const model = await modelsClient.models.get<Post>({
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for await here

@zknill zknill merged commit d710aa1 into main Oct 20, 2023
5 checks passed
@zknill zknill deleted the zak/COL-41-models-namespace branch October 20, 2023 14:13
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.

2 participants