-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Vote on moving forwards #8
Comments
I don't think a vote is necessary as to if it is the proper direction, particularly if there is just one option being considered. angular-ui can still change, improve, or replace its routing in the future. |
Well lots of people have created their own routing solutions, so this is just one more. I mean't that I alone cannot say that this is automatically the best solution out of all the ones so far. Ultimately I guess my question is who's solution do we officially adopt (even if it's just as a starting point)? If no vote is needed, then there are more than enough people who have the permission to relocate the code over to this repo. |
I know @ksperling and I kind of got on a roll (of our own) and potentially steamrolled the other solutions on the table. I personally just felt that @ksperling code was the most intuitive and feature complete solution seen yet. However it would mean a lot to me (and Karsten I'm sure) to receive a vote of confidence for his code base from those who had implementations of their own—as they are some of the best suited to make such a judgement. Or conversely I'd like to hear from those same ppl on why they may dislike Karsten's code. Karsten or I could work on a summary soon to explain the subtle nuances of the API; in the meantime reviewing his example is your best bet. I think writing docs will be the best "overview". |
Whoops wrong button |
Old saying at Microsoft: code wins. I think that in the absence of other I think the important next step is for various projects to actually use it Michael
|
@ksperling what is your email address? |
I'm happy to move the code into the AngularUI project. Should this go into a branch of 'angular-ui', or be kept separate in the 'router' project? Would it be best to copy the build / test layout etc from the main angular-ui project? The other approach that was mentioned in the discussion was to fork the Angular repo instead and aim for doing a pull request sooner rather than later. Given that the current code does not have much in common with the core Angular $router implementation, a stand-alone repo or an AngularUI branch is probably less work in the short term, without adding much additional work if/when we want to turn this into a pull request for Angular, as it would be a fairly large change in either case. On a related matter, I've been wondering if $routeProvider should be reimplemented in terms of $stateProvider, so that to migrate a project to use it one could start by simply dropping in the library and then changing things over gradually. I've deliberately kept the state object very similar to the configuration object passed to $routeProvider.when, this should be pretty straight-forward. In that respect I've also been wondering if the state-aware view directive should simply replace ng-view. I don't think it's particularly useful to be able to use both vanilla ng-view and ng-state-view in the same app, so I'm tending towards replacing. Thoughts on this? @michaelwinser I agree it's important to try this out in practice and see what works well and what needs improving @timkindberg my email address is karsten@sperling.co.nz |
Personally, I prefer my own routing approach because of the deep integration with a real statechart library (and I am using it in production, which might be a requirement for a routing solution before merging into angular-ui, or at least promoting it in any way). But I haven't had time to improve upon my solution and make a nice interface, and my ideas about what routing should be responsible for seem to be a bit different than everyone else's. The routing solution given here seems good for the community and angular-ui. |
@ksperling whatever project we go with we will be locating it on this repo, not as a branch of AngularUI. It will also not be a fork of the core for now. We will develop a polished solution on here and once it's stable enough we can look into integration. I will have to take some time to compare and contrast @ksperling's solution and @gregwebs's as I believe the core team also pointed me at @gregwebs's solution. My thoughts are do we want to have something (that eventually could become AngularJS core) that creates a dependency on a third party lib. |
In terms of external state library vs internal, my view is (and this is what I've been coding towards) that URL handling, state management itself, and view management should be as orthogonal and independent as possible, so that we can provide a core state machine implementation that caters for 98% of use cases, while making it feasible to integrate with different state libraries for more specialized use cases. Essentially the same approach that Angular has taken with $q vs Q. |
@ProLoser I've done some more work on my code, in particular around splitting up the source, adding tests, and starting to add documentation. The latest version is (still) at https://github.com/pluk/angular-extras however if there are no objections I'll push the code into a branch of this (angular-ui/router) repository in the next couple of days. |
I spoke to the core team today and confirm that they're going to need a With that in mind why don't you go ahead and move your code over here as Can you prepare a document perhaps on the wiki that breaks down in a short On a side note, although state machines are fairly a stab list I personally
|
One of the things that seem to delude me, and maybe this is because of a heavy OO background... But where does all this become "meaningful states"?... To me states gets meaningful when they provide a set of actions, and it is those actions that define transitions... On a conceptual Basis this would make sense to me in terms of state, but I can't seem to link that "thinking" to the code here... So I hope someone would be able to elaborate on where my "actions" come in to play...
Obviously the above is Bloated beyond reasoning, there is certainly things that could properly be inferred by having nested states or child states as such... A can't quite make up my mind if those to things are "the same thing" or if there is a slight difference to be harvested in the two concepts. But that doesn't matter, right now, as all I am trying to do is find the "same" concepts in the proposal here. (The above is Pseudocode, not a working example) The core thing in the above however is that I have defined Transitions (I haven't defined the logic to go with each state)... I can't seem to find where that is done in the code from @ksperling, and without that where is the state pattern?... o.O... The value of defining those transitions as they are above is that I can do different things going from one state to another on the same actions, so e.g. ArticleItemState -> ArticleCategoryState //openCategory
ArticleRecentState -> ArticleCategoryState //openCategory
In any case, I hope the above gives a good idea of what I am thinking when we say "state", and can help my brain to link that to the proposal here... |
@jeme said:
@jeme, so what is it that you are looking for exactly? The ability to do different things while entering a state, depending on which state you are coming from? I don't think this functionality is built into @ksperling proposal; or if it is I have not seen how to do it. I like this though, and I know there is an You also propose new pseudo code for a more OO approach (similar to ember). Are you using this to just illustrate your point or are you calling for a new API that is more OO? I'm not sure I'd want a much more OO approach, I like the current API, but am interested in any important features that may be missing. On a different note, I liked certain aspects of @gregwebs state code. In particular I liked the @ksperling does your implementation have the ability to stop someone in their tracks and force them to log in if they try to access a state that lives within a "logged in" state? Would this "logged in" state be an |
We don't need to hold up this project because it doesn't have a proper state machine since it is still an improvement over the original. I can try to take a closer look at the possibility of integrating StateTree this weekend. @jeme rather than define transitions in StateTree a user defines enter & exit callbacks on states with the assumption that almost all state transitions are valid since a user can always navigate from one part of the app to another. The main exception is the initial login which I added |
@timkindberg One of the most valuable features in a state pattern to me is that I can do different things depending on the transition. The code I posted was merely to illustrate what I meant, and it properly doesn't do it as well as it could. So it wasn't a proposal for an API change, but more a "how would I/we do this in this API"... It also stems from the current approach I am working on... Some points I like from where I am currently at is:
It is VERY OOish now though, and I would love to leave that behind in the dust.. but right now I have been working on the concept for my own blog, so I could always refine it. @gregwebs Yes, but that doesn't sound exactly like what I am after... Because that behavior is bound to the state in my head, and not the transition, so you can't alter that behavior easily depending on what state you arrive from, it would always be the same behavior. (If I have understood it correctly)... |
State machines are orthogonal to OO, so I'm not really sure what this is supposed to mean.
I'd be +1 for this, because then you can attach as many handlers to as many arbitrary transitions as you want. Once we're more comfortable with the API, maybe we can come up with a syntax that doesn't require as much manual dispatch hand-waving. I've seen SM libraries in Ruby that do things like |
That MY background is mostly from the Object Oriented paradigm, and so I might have difficulties mapping the way I am use to implement and treating state patterns in an Object Oriented fashion to what have been proposed here and therefore have a hard time seeing how I obtain the same possibilities and values, those that I then after mention... But BECAUSE of my Object Oriented thinking, they might actually be possible and already there, but that I just can't identify them... |
@jeme, in response to your main points... 1.
@ksperling implementation definitely doesn't separate the two out completely, but I'm not sure this is necessary. And as I read somewhere recently "There is no route without state", but there could be states without routes. 2.
We don't have anything like that at the moment. Maybe something like this?
Though this would cause the url to be redirected, so the /comments part would disappear. So perhaps we could do something like this:
Anyway, I think we can work all of this type of thing in if needed. Also $anchorScroll would probably be used in this specific example but I can still see the potential coolness of executing a controller scope action. 3.
I don't think this will be needed in @ksperling implementation. It's a nice feature of your API but you should be able to do what you need to in ours. 4.
If we attach "actions" to controllers then we'd have this ability too. Lastly...One question that I asked earlier and I definitely don't want to get lost is:@ksperling does your implementation have the ability to stop someone in their tracks, and for example, force them to log in if they try to access a state that lives within a "logged in" state? Would this "logged in" state be an |
1.True, it's a nice to have, it does give the opportunity to improve in things more separately. I have always been a big fan of separating concerns. But it's not a major deal. 2.I assume that one would Execute the parent state first if needed. In my case I would obviously just execute it on the state and then return that same state in a very OO manner:
Which evidently means that any actions will ofc. be able to take any dependencies you wan't... So it could delegate it on to a controller if need be. But I chose the state here as I don't require a controller or template, again because a state can be a simple action in it self... So it might be more like:
3.Well again, this might be damage from many years in an Object Oriented Paradigm... But what makes this different that simple nested routes then?... I'm stuck on this because it makes little sense without it... Currently I only utilize it for a minor things where it saves me replaying the template when I transition between states that uses the same template. 4.Ok, strike that one then. |
@jeme A lot of comments to respond to! Maybe to start off with I'll just summarize what a "state" as I've implemented it currently is:
|
@gregwebs The current implementation contains some changes based on some of your earlier feedback, and I'd be keen to hear your thoughts on those: It's now possible to use strong references to states rather than names, for example you can do
If used in this way, the state object is not modified by $stateProvider at all, and what's exposed as $state.current is exactly that identical state object that was registered. This means the state objects don't need to be anonymous blobs of JSON but can instead be objects produced by a library like your StateTree, or have additional application specific methods or properties. In addition I've been thinking about allowing an optional strategy object to be placed between the $stateProvider and the state, such that rather than accessing a state |
@jeme to respond to your 4 key points briefly directly again:
In my code they currently are. "Routes" (I've called them rules to avoid some confusion with existing Angular routes) are handled by $urlRouter and are simply rules that get matched against $location. Because wanting to have a URL that causes a transition to a particular state is very common, you can use state.url and $stateProvider will add a rule that calls $state.transitionTo() for you, but that's no different from adding that same rule yourself. And of course you can add any number of other rules to $urlRouter that do other things, or act on $state in any other way you like.
The way I design applications, URLs are essentially nouns or "place names", not actions / verbs. $state has no built-in concept of 'actions', but there's nothing stopping you from adding arbitrary methods to your state objects and calling them via $state.current.yourCustomMethod() either "manually" or from a $urlRouter rule.
See previous answer.
Yes. You can also trigger state transitions without URL-rules, via $state.transitionTo(). For example if I wanted to implement a wizard UI that the user has to go through step by step and can't bookmark, I would assign no URLs to any except the first state of the wizard, and transition to them via $state.transitionTo('wizard.step2'); |
@ksperling wow I feel like this clears up so much. It also solidifies my confidence in your implementation. |
I vote that we use everything you just wrote (edited a bit) as the readme! |
I'm wondering if we should rename 'state' to 'place' in the code, i.e. $placeProvider, $placeParams etc, to make it clear that this state machine is only responsible for managing coarse UI state, i.e. the "place" within the application and the what UI belongs to it? I like calling things by their commonly understood name, and these 'states' form part of a hierarchical state machine, so "state" and related terminology do seem applicable, on the other hand maybe it invites too many assumptions? GWT uses the term "place" in a similar (but not identical) way. Possibly something to have a vote on later... |
I've pushed the latest code to the ui-states branch in this repo, and also updated the sample to show navigating with $state.transitionTo() and states that don't have a URL. This version now handles updating $location when a transition is made via transitionTo(). |
@ksperling Let me try to clarify some more since you clearly missed a few points. Let me just start by removing this one:
That works with what I have as well, it is very flexible in the way you wan't to implement it however.
Yes, but that is a more simple case, and what I am saying I am doing is not what you describe here... Say instead of just
Note how A. I misspelled contacts in the definition of category, I assume that fucks up the states yes?.... (no I don't like that strings has this sort of meaning, sorry) Anyways... More importantly B. What I do is give control over what happens when going from
All that flexibility comes at a price right now though, and that is allot of "boiler plating" which would be lovely to get rid of, which this solution had less of, so I was hoping that when you called it states, it actually supported what I understood in a full state pattern. |
The objection around typos was already addressed -- you can use variables for your states, and then use these variables to reference the state, e.g.
What if you mistype a URL pattern though, or a service name, or a template URL, or a directive name in your html? They all cause an app to misbehave at runtime... I don't think my proposal is worse in that sense than many other APIs in angular. In terms of part B, I can see that there might be a desire to retain an individual view between states, even though I'm not sure I really understand your example -- you've defined the two states to have the same template, but no controller, so how do they differ at all? The common view could also be factored out into a new state that is a parent for both of the states in question. The reason I have stayed away from trying to retain views between states that are "similar" is because essentially impossible to compare. In your case the template is a URL, so its easy to compare, but what if the template is defined via an asynchronous function? Then there is no way to tell if they are the same template without running it. Checking if the controllers and all the resolved dependencies are the same would be even more complicated. I've re-read your other post regarding the actions - I agree it may be useful to know in onEnter/onExit what the previous/next state is, but I don't think there is anything to be gained by forcing every transition to be manually specified via some sequence of actions. Can you provide an actual use case that the current approach doesn't support? Jens Melgaard notifications@github.com wrote:
|
@ksperling There is a bit of a difference, if you typed the URL pattern wrong the resource you where looking for would merely be in another place than where you looked. If you typed the template URL wrong you would get an "resource not found" response from the server... Both are gracefully handled errors... Question is... If you mistype the parent in a state, and that parent does not exist... how will you framework behave?... But I made it clear that it was something I didn't like... meaning that it was an opinion. And missed that second part since it is inline (replying form mail seems to have it's disadvantages it seems).
There is no comparison, that's the point of having the transition.
Again... Very OO style way of explaining it... But it gives a more fine grained controll over what happens when I go to the ArticleCategoryState when I am in the recent state as opposed to when I am in the Article State. When coming from a completely different area we could deal with it in 2 ways... Either have the "OpenCategory" implemented in a MainState, or simply start by taransisioning to the ArticleState, then to the CategoryState.
Absolutely concur that there is no reason to force every transition to be manually specified as it creates a huge amount of boilerplate as mentioned before, so if it could be an "obt-in" that would be perfect... It becomes difficult to see how that would work however. It's not that we cant add actions like so: And somehow maybe execute those through an Execute method instead, but there is a long way from there to making them usefull I think... Yet I am not sure.
Alternatively specifying transition handlers maybe...
Maybe that needs to be elaborated a bit in terms of Pre, In and Post... (before, between, after)... |
On Feb 14, 2013, at 2:42, Jens Melgaard notifications@github.com wrote:
This is JavaScript, not Ruby. Get over it. |
That doesn't mean we should be embracing a bad practice... And ill refrain from being rude back at you, as it's not really constructive, but continue the path and I won't even bother with you comments. |
Jens, I agree that the string usage is prone to error and Nate definitely You could always use constants instead of strings and Karstens Thanks, (Sent from my mobile) On Feb 14, 2013, at 8:46 AM, Jens Melgaard notifications@github.com wrote: This is JavaScript, not Ruby. Get over it. That doesn't mean we should be embracing a bad practice... And ill refrain — |
@jeme What you don't seem to understand is that best practices are all about context. What's best practice in one language or context can be counterproductive or just plain wrong in another. That said, there are plenty of safety checks that could be implemented with string-based states, but it's wasteful to make assumptions about problems we aren't even sure exist yet. This back-and-forth has dragged on long enough, with no useful outcomes. Let's settle it, move on, and start using the API. If debugging mis-typed states turns out to be a legitimately big deal, no problem, we'll come back and solve it. But let's not waste time trying to predict the future. |
Strike that, one useful outcome: documentation! :-) |
That a state has a name isn't what i consider the bad part... that if the name has a part before '.' that when matches another states full name means its a child, that's where it becomes bad practice to me... If history tells us anything, these type of things should be avoided... That said, this can be wrapped away behind another interface... So lets let that be for now... @nateabele And what you don't seem to understand is that it was an opinion... |
@jeme No, it really is an opinion until someone is actually using the API to build an app and says "Shit! I just mis-typed a state name and that was really hard to debug!" At this point, it looks like you're just arguing for argument's sake. If you think this is a legitimate problem, then let's see a patch. Otherwise, let's move on. |
@nateabele And it looks like your just jumping in to troll here and there... So sorry... I won't pay attention to your comments anymore... After all, you seem to be more concerned with going after the person than the ball... Also, your post doesn't make sense anyways, I SAID it was an Opinion, I never denied that... I even had to make that explicit over several posts... you still seem to have a hard time to get that... And I can respect that you are A-OK to use strings like this, it doesn't mean I have to agree... |
@nateabele you are introducing a rude tone and de-railing what was a polite and productive conversation that was coming to a close, the opposite of your intent. Your comments aren't constructive here because nothing is being held up. We are all green-lighing this routing implementation, just providing feedback. @ksperling only need address comments here if he wants to. It would be better just to un-notify yourself if you are tiring of the conversation. |
@jeme Then I misread you. My mistake, I apologize. In any case, if you still feel this is a significant issue, I whipped up a possible implementation, which keeps the usage of string identifiers to a minimum, without compromising the existing API: http://plnkr.co/edit/V7bFluzNetmneBXZ4fsA Here's a snippet of the example above, adapted to use it:
It requires minimal hand-waving compared to just using strings, with the added benefits of (a) being able to attach more things to states (I'm not directly suggesting anything here, it just keeps the API flexible for future ideas), and (b) being able to more explicitly infer things about states, i.e. Hopefully this is a worthwhile compromise to allow both string and object states. If everyone agrees, I'm happy to patch @ksperling's work to allow string-based states, as well as the above state object examples. |
@nateabele I'm not too fond of that change, I think I would vote no on it (thought I love the ability to access parent and root etc). Also you are being rude, but you have really good points and add value, but just be a tad nicer ;). I agree with you that we need to keep moving forward and it seems like the conversation is starting to go back and forth. @jeme is still trying to wrap his head around our way of doing things, and his confusion and lack of comfort with the current API has brought up some very good conversations and explanations. It also is seeming like @jeme doesn't fully understand the current implementation and may need to re-review it. I personally am feeling very comfortable with the current code and love the API for the most part. @everyone, I do think its time to move on as accepting @ksperling code as the way forward. So from this point on I think everything issue item should be addressed as either a feature request or a bug. We should start using the github issues to keep track of each item individually. Use the New Issue to start a SINGLE MINDED thread about a SINGLE topic. Examples of issues at this stage would be:
Then people can discuss and decide if it will be implemented or not (does github issues have vote up/down feature?) if not we can do a +1/-1 comment. Karsten can then assign himself the work (or other brave contributors). |
Closing because we are moving forward with @ksperling implementation, if you need to keep any issues from this discussion alive please make a new independent issue. @ProLoser Sorry if closing things down seems rude, please reopen if I'm being trigger happy here :) |
I realize I have been fairly inactive and probably am the least-knowledgable individual when it comes to how state-machines work. My only major concern is making the API as intuitive to use as angular's current routing api (or more-so if possible). I am still on the fence on wether I personally would prefer a string definition or object definition, but for now I agree that if 1 of the two solutions has already been developed we should get that working and review a refactor. The API should still be considered highly alpha and malleable to refactoring even post-commiting. I am hoping that eventually this new router will be immediately comprehendible to people who don't even know what a state machine is once it's moved along. |
@ProLoser @timkindberg One thing I wanted to clarify is that my proposal was to allow object-based state definitions in addition to string-based definitions, not to their exclusion. @timkindberg Let me know if that changes your opinion at all, and I'll open a new issue on it. |
@nateabele I do like the idea, just unsure about the implementation you proposed. I think it's worth opening an issue to hash it out. |
@nateabele I think a new issue may not be necessary actually. I believe we already have an OO approach available. Well... object-based I should say... not really OO. See here:
We could still write up an issue to explore this though if you want to. If it gets no traction then so be it. |
@jeme Just to clarify in case you haven't tried it by now or reviewed it in the source, the current implementation does catch mistyped state or parent state names (even when implied via "." separator in the name) and throws an exception. @nateabele Yes that's essentially what you can do in the current implementation, even though I would probably refrain from nesting the definitions in that way, given the potential of a state name part conflicting with a property that $stateProvider recognizes (e.g. in your example what if we decided to make 'list' have some meaning for $stateProvider in the same way 'view' or 'template' do now). One option to avoid this would be to prefix all the API property names with '$', i.e. '$views', '$templateUrl' etc however that feels like adding a fair amount of "line noise" for what may not be a large benefit. That said, it would make it easier to add methods and other things to the state that have purely application-defined meaning, without fear of trampling on something that is meaningful to $stateProvider. However as far as I can see that's not an approach that Angular takes in any of it's existing APIs (e.g. route definition or directive definition). So I'd tend to wards not going down the '$' prefix path at this point. @* I think the code is at a point where it works well enough to be able to try it out "in anger" by trying to refactor some small/mid-sized existing apps you've got over to it, and see what real world issues comes up. A contrived sample app really only goes so far... What I'm going to look at next myself are
|
@ksperling I'm assuming you were replying to me and my example of object-based states as well... Also I'll be honest, I don't get bullets 2, 3 and 4 of what you just wrote. Obviously you are free to go experiment with whatever and owe nothing to us (you've given so much as it is), but if you did want help brainstorming and coming to a community-driven consensus on any of those individual issues you just listed, consider creating issues for each one. I realize that its probably a lot easier to just go do it though... ;) |
I'm just going to create some issues for each of those bullets, then people can comment on them and you can mark them as done, etc. |
@timkindberg Sorry missed the @tag then, yes, I did mean to include you I agree it's a good idea to start splitting things up into separate issues, these long threads get rather hard to follow for anybody coming along later |
@ksperling Nope, I shimmed the code and couldn't find anything indicating that an error was thrown on the particular issue. After running the code I can see it states that "Uncaught Error: No such state 'contactst' from sample"... I think it would be really helpfull if it was more precise... The above doesn't really get ones mind at that it is a pointer to a parent state that is the issue. Maybe something like: "Could not find parent 'contactst' for state 'contactst.list'... It might involving catching that specific error and throw another one, but since this would be an "Exception state" I think the overhead of that would be ok. |
I'm happy to see @ksperling and @timkindberg have been moving this along.
I started this project because I wanted a routing solution that AngularUI could officially stand behind rather than have 20+ solutions that I couldn't really support or suggest to people.
That being said, I briefly spoke to @nateabele and he seems to think you guys are heading in the right direction.
I have 2 proposals:
1. We vote on if this is the proper direction
@ksperling and @timkindberg if you'd like to briefly summarize the new proposal here for everyone.
Since I haven't been able to dig into the solution yet, I'd like the community to decide if this is the right direction
If it is...
2. We move the project to AngularUI (?)
This of course may be rejected, as everyone is entitled to own their own project.
However I would like to have an official AngularUI project for routing that we can help review and collaborate on. @ksperling as with all AngularUI projects you would still have control over and be able to manage the project, but this way we can get a bigger team on board to actively help you once this solution takes off.
We would be able to help clean up the project and bring up code quality standards as well as tie it into the AngularUI which will help with getting the word out!
The text was updated successfully, but these errors were encountered: