Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

simplify main api (closes #155) #154

Merged
merged 24 commits into from
Mar 9, 2018
Merged

simplify main api (closes #155) #154

merged 24 commits into from
Mar 9, 2018

Conversation

giogonzo
Copy link
Member

@giogonzo giogonzo commented Feb 26, 2018

rough list of changes (all breaking!):

  • import { x } from 'avenger/lib/graph' -> import { x } from 'avenger'
  • import { x } from 'avenger/lib/cache/strategies' -> import { x } from 'avenger'
  • removed make function: no need to "make" anything
  • removed the need to ... spread the result of import * as queries from 'queries' before passing it to the context provider
  • changed apis for all the functions accepting queries as input (query, invalidate, runCommand, extract/restoreQueryCaches)
    • now they all accept queries as a { query1, ..., queryN } dictionary instead of graph + Ps string ids
    • the string id remains as an optional prop in Query({ id }) just for debugging purposes

commits should make sense if reviewed in order

@nemobot nemobot added the WIP label Feb 26, 2018
@giogonzo giogonzo changed the title simplify main api simplify main api (closes #155) Mar 2, 2018
@giogonzo giogonzo requested a review from veej March 2, 2018 09:19
@nemobot nemobot added in review and removed WIP labels Mar 2, 2018
@@ -17,7 +17,7 @@ export type QueryArgsNoDeps<
A extends IOTSParams,
P
> = {
id: string,
id?: string,
Copy link
Member

Choose a reason for hiding this comment

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

Why optional?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it is still used as the name for the caches used in each node, and this can be helpful when debugging with DEBUG=avenger*: the logs are colored and organized by name of the cache, and the default name if not passed is just "anonymous".

We could make this more explicit and call it "cacheName" maybe?

Copy link
Member

Choose a reason for hiding this comment

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

or debugName... up to you

Copy link
Member

@veej veej left a comment

Choose a reason for hiding this comment

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

💯

…d nodes

(not recursively all "required" nodes)
@giogonzo
Copy link
Member Author

giogonzo commented Mar 9, 2018

@veej please take a look at the last 4 commits

@giogonzo giogonzo requested a review from veej March 9, 2018 13:41
Copy link
Member

@veej veej left a comment

Choose a reason for hiding this comment

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

💯

@giogonzo giogonzo merged commit 5c4efc2 into master Mar 9, 2018
@nemobot
Copy link

nemobot commented Mar 9, 2018

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

Successfully merging this pull request may close these issues.

None yet

3 participants