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

Implement nav/datafy for pull #325

Closed
wants to merge 4 commits into from
Closed

Conversation

IamDrowsy
Copy link
Contributor

@IamDrowsy IamDrowsy commented Oct 23, 2019

The PR contains a version of pull/pull-many that decorates the returning maps with implementations of datafy/nav protocol.

When navigating along ref or _ref fields, it pulls the referenced entities from the db.
Currently, when navigating, it always pulls all fields (including the reverse refs) to ensure you can navigate back and forth.

I'm not an expert in implementing datafy/nav so feedback is very welcome.

They could replace pull/pull-many of the core. As there should be no difference if you don't call datafy on the result.

For now I have not looked into ways to make the result of queries navigatable.
This touches #313 .

They work as pull/pull-many but return maps that impement the datafy/nav dance.
@IamDrowsy IamDrowsy changed the title #313 Implement nav/datafy for pull Implement nav/datafy for pull #313 Oct 23, 2019
@IamDrowsy IamDrowsy changed the title Implement nav/datafy for pull #313 Implement nav/datafy for pull Oct 23, 2019
@tonsky
Copy link
Owner

tonsky commented Oct 24, 2019

Shouldn’t it be on entity instead? I can see how to make entity navigable, but pull results are often messy, can have multiple entites and arbitrary attrbutes mixed?

@IamDrowsy
Copy link
Contributor Author

Yeah that sounds more reasonable. I will have a look.

@IamDrowsy
Copy link
Contributor Author

I updated the PR to extend the Entity type. When datafying an entity it still uses the pull api under the hood to pull all attributes (including reverse attributes).
To use it, you currently would have to require the datascript.datafy namespace manually.

The implementation could be included into impl.entity, but this would introduce a dependency for entity to pull-api.
So it probably the best way to make it available everywhere would be to just require datascript.datafy in datascript.core.

Btw. I also tried to have a look how datomic does implement nav/datafy (regarding what to pull etc.) on their entities, but this is not included in datomic-free.

@tonsky
Copy link
Owner

tonsky commented Oct 25, 2019

Thanks! I’ll take a look later

@thenonameguy
Copy link

thenonameguy commented Nov 4, 2019

Hey @IamDrowsy, I wanted to create a quick WIP for the same issue, your entity datafying is way better than mine, but maybe you could borrow the DB/Datom datafy code from mine:
master...thenonameguy:datafy_nav
Feel free to include my code.

Thanks for the awesome work on this!

@den1k
Copy link

den1k commented Jul 11, 2020

Merge ready?

@IamDrowsy IamDrowsy marked this pull request as ready for review August 2, 2020 10:07
@thenonameguy
Copy link

@tonsky if you have got the time to review & merge this, it would be greatly appreciated!

@tonsky
Copy link
Owner

tonsky commented Dec 29, 2020

I’ll take a look this week, sorry it was lost for so long :)

@tonsky
Copy link
Owner

tonsky commented Feb 13, 2021

Sorry it took so long. The PR is fantastic, merged fully, deployed as 1.0.4. Thank you @IamDrowsy!

@tonsky tonsky closed this Feb 13, 2021
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.

None yet

4 participants