Skip to content

Add provenance tracking to select and de-select nodes#122

Merged
JackWilb merged 8 commits intomasterfrom
provenance
Dec 17, 2020
Merged

Add provenance tracking to select and de-select nodes#122
JackWilb merged 8 commits intomasterfrom
provenance

Conversation

@JackWilb
Copy link
Copy Markdown
Member

@JackWilb JackWilb commented Dec 7, 2020

Depends on #121, I'll retarget this branch to master once it's merged (this just helps with seeing the changes).

Adds the most recent version of the provenance library that I've been told "will be released without changes to the API". That means the logic won't have to change for us to use the 2.0.0 release of the library.

I think the provenance library cannot track objects that are vue reactive, because it will get into an infinite loop. To get around that problem, I read the state in with JSON.parse(JSON.stringify(state)) to get rid of the reactivity and then I make sure when I update, I don't pass a reactive object. Kiran told me "Deep-diff and trrack both work with only pure JSON objects... I added serialization support for Sets", so passing a set through as I have done, should be okay.

You'll find that the first thing that I added to the provenance is selecting and deselecting nodes. I figured I'd keep this change small, and I'll address the other pieces that we would want to update with provenance in a follow up. We can also implement a provenance visualization that has been created by our team at Utah that works on Trrack.

We can also support loading an entire state from a provenance querystring, if we'd like. So that's something to consider for additional follow ups.

@JackWilb JackWilb changed the base branch from master to re-architect December 7, 2020 21:01
@JackWilb JackWilb changed the title [WIP] Add provenance tracking to select and de-select nodes Add provenance tracking to select and de-select nodes Dec 7, 2020
@JackWilb JackWilb requested a review from waxlamp December 7, 2020 21:10
Base automatically changed from re-architect to master December 9, 2020 01:10
@JackWilb
Copy link
Copy Markdown
Member Author

JackWilb commented Dec 11, 2020

This is waiting on an update to the Trrack library that will add types for deep-diff. We're the first typescript project to use "strict": true in the typescript compiler, and thus disallow implicit anys, so the first project to catch this issue.

},

createProvenance(state) {
state.provenance = initProvenance<State, ProvenanceEventTypes, unknown>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This goes somewhat along with my last comment: Since the provenance is for an independent system that reacts to changes in the state (and possibly causes some changes of its own) from an engineering perspective it seems like it should be encapsulated outside of the VueX state.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we follow your suggestion from above, we can have this be a method in the provenance "component" and then call it from wherever we bind data to the provenance (I suggested App.vue above).

For now, I can move the initialization out to the provenanceUtils.ts file, if that sounds like a reasonable compromise

Copy link
Copy Markdown
Contributor

@waxlamp waxlamp Dec 16, 2020

Choose a reason for hiding this comment

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

You can move this function if you want, but it's not necessary right now. If you want to file an issue to re-examine the design later, that would be more than enough to get this PR out the door.

Let me know what you'd like to do here, and then I can resolve this thread.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Opened #137

waxlamp
waxlamp previously approved these changes Dec 16, 2020
Copy link
Copy Markdown
Contributor

@waxlamp waxlamp left a comment

Choose a reason for hiding this comment

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

Pre-approving even though there appears to be a breakage caused by trrack. Once that's fixed I can re-approve.

@JackWilb JackWilb requested a review from waxlamp December 17, 2020 05:45
@JackWilb JackWilb merged commit e2099a1 into master Dec 17, 2020
@JackWilb JackWilb deleted the provenance branch December 17, 2020 16:19
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