-
Notifications
You must be signed in to change notification settings - Fork 125
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
Facet name change #2
Comments
Hi @idream3 , I agree with you on this one. Facet is kind of a weird name. Though "getter" is more to the point it still does not quite explain what it does I think. Technically it redirects the path of a state to the function defined. So words like: redirect, map, proxy etc. comes into mind. Actually I think proxy is the perfect technical term for it. As you are redirecting to a different source and receiving its value. What do you think? |
You know, I think '.map' is the way to go. It is a known concept for arrays and observables. And actually a facet is much like an observable. So yeah, I have landed on 'map'. Seems okay? cerebral.map('myState', [deps], function () {}); |
Happy with 'map'. I think it is expressive. A couple of other things to consider:
var state = {
todos: [],
newTodoTitle: '',
visibleTodos: {
map: ['todos', 'newTodoTitle', function(state) {
return state
}]
}
}; or var state = {
todos: [],
newTodoTitle: '',
map: {
visibleTodos: ['todos', 'newTodoTitle', function(state) {
return state
}]
}
}; |
Hi @idream3 ,
var state = {
todos: [],
visibleTodos: new Cerebral.Map({})
}; That way the logic could just check for that type of object, but it still is really complex when you consider the possibility to run So yeah, the idea is good, but the implementation complexity is just not worth it in my opinion. It would also be difficult to read the tree in large applications. |
Hey @christianalfoni ,
Cerebral.Map('myMap', ['path1', 'path2'], (cerebral, state) => {
state.path1 // value
state.path2 // value
cerebral.get('otherPath')
cerebral.signal.someSignal()
}) Maybe at some point we want to access
To answer this question I decided to run some examples with actual data from my application. It is a very large flux application and is a perfect real world case, beyond trivial todos, to test this out. Below you can follow, in order, some of that process.
End result is that I definitely agree not to declare entire Maps straight on the tree unless they are one liners. However, I see much merit in defining what is a Map directly on the tree. Number 4 is my current best approach var state = {
todos: {},
visibleTodos: Map.visibleTodos
};
// returns an array of paths with last item a function.
// This is mapped to Cerebral.Map('visibleTodos', ['todos'], (cerebral, state) => {})
var Map = {
visibleTodos() {
return ['todos', (cerebral, state) => {}]
}
} When the tree is initialized, paths that are functions can be deleted out of the tree and run through Cerebral.Map(). This way you can still use I like being able to use dot notation when specifying a key path, that way I can segment my raw application data to my view specific data (as shown in gist 4. above). Cerebral.Map('view.selectedUser', ['data.users', 'view.selectedUserId'],
(cerebral, state) => {
return state.users[state.selectedUserId]
}) Like i said, I am trying to understand how this tree fits with my current large data set (those examples are a fraction of the total set) without being overwhelming and messy. You can see some of the complex relationships in the gists. |
Thanks for great input and spending time on this @idream3! Your suggestion on allowing functions in the tree makes sense. The really great thing is that it makes things simpler. You do not have to compose map methods to the state tree in your head, just look at the state tree, everything is there. Really good. I will work on an implementation of this, but I think we need to change the signature a bit. The reason is that the var state = {
todos: {},
visibleTodos: Map.visibleTodos
};
var Map = {
visibleTodos() {
return {
value: [],
deps: ['todos'], // or key/path map
get: function (cerebral, deps, value) {
}
};
}
} I agree that dot notation is nice, but technically a key can have a dot in its name, though it is very unusual to do so. I do not think it is a good idea to allow both array paths and dot notation paths though, it should be one of them. It is a matter of consistency I think. The problem with dot notation is that you are not able to do this: let todo = cerebral.getByRef('todos', ref);
cerebral.set([todo, 'completed'], true); You would be forced to change your dot convention in certain situations. So the array is more flexible than dot notation. It also allows you to easily "produce" a path. So yeah, I am not completely sold on dot notation, but I am on your suggestion on map ;-) Please give some feedback on the suggested final signature above and I will get to work on it! |
Hm, thinking of it, the last example could be solved with: let todo = cerebral.getByRef('todos', ref);
cerebral.merge(todo {
completed: true
}); Hm, okay, but I still think there should only be one implementation. I prefer dot notation too, it just reads a lot better. You see any use cases for array? |
Not immediately, without having real world experience. I plan to compose part of our application in cerebral in the coming week to see how it goes. I like your signature, devs can create their own abstraction to compose a simpler form if desired. It also means at the outset it is more readable. |
Though I like dot notation the new reference implementation quickly requires you do use arrays. I will keep arrays for now. Please pound on me if you still want dot notation ;-) |
Hey, hope I am not being presumptuous! :)
But what is a facet anyway? http://dictionary.reference.com/browse/facet
Closest I could come to in this context was number 3. aspect - as in, an aspect of the data tree.
What are facets doing? getting, combing, composing, transforming, reducing...
Personally I like the word 'getter' but maybe we could create a new one? what is your take on facets?
Close this thread if it is irrelevant :)
The text was updated successfully, but these errors were encountered: