-
Notifications
You must be signed in to change notification settings - Fork 1.8k
JS: Add support for vuex #6213
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
JS: Add support for vuex #6213
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vuex.qll
is quite a mouthful.
(Everything outside of Vuex.qll
looks fine).
I'm not done reviewing it, but here preliminary review.
Thanks for the initial review @erik-krogh, I know it's a heavy PR to work through. ✨ I've addressed the comments so far. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments, but nothing big.
Looks good 👍
Co-authored-by: Erik Krogh Kristensen <erik-krogh@github.com>
Adds support for
vuex
, a Flux-type state management library for Vue.It is similar to Redux but was simpler to model as more of the core functionality seems to delivered out of the box, so things are a bit more consistent and fewer third-party libraries needed to be supported.
The model is based on API graphs, and there were a few tweaks needed to make this work nicely:
getAValueReachingRhs().getStringValue()
can be used get string values reaching a def-node. (This landed separately in JS: Treat string literals as SourceNodes #6192)The
Vue
model is not yet based on API graphs, so I only did a very loose integration, so as not to make the eventual migration harder.Evaluations: (both containing a slightly older version of the string literals as source nodes PR causing some spurious alert diffs)