-
Notifications
You must be signed in to change notification settings - Fork 25.5k
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
Elvis operator in expressions: ?.
#791
Comments
What are the reasons for not making this the default behavior (similar to angular 1)? |
More expressive. It hints to the reader what may be lazy loaded and what may not. Example: |
Feels kind of weird to me; it's surprising to have language extensions within the binding expression syntax. This might also need some more thinking, but I don't see this as being that useful. If angular defaults to gracefully handling null dereferencing in expression then the developer doesn't have to care whether something is loaded lazy or not. The times when people do care is when something needs to be shown in place of the pending data, which will still be handled with an |
Yeh, I might be used to Angular 1.x ways of doing things too much, but I find forgiving evaluation of NG 1 expressions quite handy. I'm not sure if I like the idea of being forced to mark all the optional parts in the expressions: this does things more obvious but more laborious as well. |
I think the explicitness of |
I am in favor of having the safe navigation operator (a.k.a. elvis operator). It exists in C#, Swift, Groovy and others, and will likely be added to Dart. So I don't think there is going to be any confusion around it. It will also allow us to retain the JS and Dart semantics of expressions without this operator. Finally, it will result in smaller and more efficient change detector code, as EDIT: updated the post with correct references to the prior art. |
@yjbanov This is different from a null-coalesce operator, though. The null-coalesce operator in C# and Swift is This can already be accomplished in JavaScript with the OR operator: The question here is about the nullsafe dereference operator Did you have some case or scenario where explicitly using nullsafe dereference would be advantageous to making all dereferences nullsafe by default? |
My bad. I meant safe navigation operator, a.k.a. the elvis. I will edit my original post. The point still stands. |
Advantages of having this operator:
I do realize its disadvantages though:
I believe the advantages outweigh the disadvantages, so I think we should include it. |
I see additional disadvantages:
I also would predict that the majority of developers would want the nullsafe behavior by default, vs. throwing an error. Perhaps this could be some kind of application-level configuration? |
I tend to side with @jelbourn - Angular 1 did excellent job at making "the right thing most of the time". Using it felt natural and enjoyable from the start. I feel like introducing I totally agree that having null-safe behaviour by default creates a (small) sharp edge and people might got hurt from time to time (things failing silently), though. |
@jelbourn, all three points are along the lines of "this is different from what people are used to and they will have to adjust". I do not see this as a big problem. It's pretty easy, IMO, to get used to typing @pkozlowski-opensource, how is In dev mode Angular could point to the exact location in the expression that caused TypeError/NoSuchMethodError, so it's easy to either change |
Coming from Angular 1 World I see it as an unnecessary burden, compared with the listed benefits. Then again, maybe I've spent too much time writing 1.x code to be objective here. |
so it's basically between either hypothetical performance improvements / simplifed code, or making upgrade path easier. Would the change detector code be just as fast/small if every field in an expression was
|
@pkozlowski-opensource, you don't have to think. Just use @caitp, there are two aspects to the upgrade. There's the mental migration from Angular 1 concepts to new Angular 2 concepts, which we discussed in detail above. The second aspect is the actual code migration, but it is pretty straight-forward - you replace every |
@yjbanov of course "thinking" part is more important / time consuming than "typing" but proposing "use ?. everywhere, like you used ." would make all the expressions look really ugly / less readable (personal opinion). So I would have to still add them selectively (and here comes the thinking part). So maybe an app-level global setting is a way to go? Anyway, I guess I got my opinion / arguments clear, not sure I could add anything more... Apparently people disagree on what majority developers want, so I can only speak for myself... |
Unfortunately, I don't think a global setting will work as you want to be able to consume third-party components without breaking them. It would also kill the explicitness in the proposed syntax. This is something we have to decide now. |
Oh, we can't have a switch, you are totally right. |
Any way to have an option on the template annotation to opt-in to it?
|
I think having a template switch to make |
I think having the template switch is worse than having the operator. We should aim for readability, and this means we should avoid non-locally defined semantics. If I'm reading someone else's code, I shouldn't have to check their options to know if the expressions are null-safe or not (especially when doing code reviews where review tools collapse full file contents). So I think our options are:
I'm still worried about code size. Nullsafe expression
Maybe there's a better way. In JS you can take advantage of truthiness and make this expression shorter, but it would still be longer. |
I have to agree with @pkozlowski-opensource, the forgiving nature of Angular 1 expressions worked well, and should be kept. Note: Adding |
We would fallow this proposal: https://github.com/gbracha/nullAwareOperators/blob/master/README.md |
My team has been bitten during refactorings by the default behavior of 1.x (renamed some property but get no warning when rendering the template). Would love to see that default change and have |
@ewinslow, this is a great point. The tools should be able to produce static warnings for both null-safe and strict property access when you are accessing typed objects. However, POJOs and maps would still have this issue. |
It seems that type-checking template linters (none exists that I know of, yet) would have an easier time with the 1.x behavior. I think we would all like IDE or gulp/grunt linters that could plug into the TypeScript compiler service and validate Not that this would be impossible with the nullAwareOperator. It would just be easier to implement such a tool without the explicit nullAwareOperator. |
@martinmcwhorter IDE support (or general toolability) is one of the goals of Angular 2. This is also why we are making sure that Angular 2 compiler works outside the web browser. This means that IDEs would be able to reuse the code that Angular itself uses to interpret the expressions. |
@yjbanov sounds good. I am a bit off topic at the moment. Right now a hole in using statically typed data/message contracts is in the templates. With TypeScript breaking changes can easily be detected in CI. Unfortunately this falls down when the types are expressed in templates. The ability to have tooling to compile these expressions and then validate them against the TS compiler service would fill a large gap. |
@vicb I meant that as a response to why we should add it. One argument was that angular was extending javascript which is true since it's surprising to the developer to learn about these extensions in the binding syntax. I was only suggesting that the decision to add extensions has already been made with Pipes. Pipes is not javascript syntax and it is surprising to the developer rather than assuming it's just javascript because it's not. I was saying that argument only stands ground if you also argue to refactor pipes (and elvis) into a macros then it's simpler for the developer to reason about why we have new syntax (because they're just macros). The macro thing is whole other argument which is why we should just add elvis since it's not breaking any of the assumed angular conventions (because we already have Pipes) |
Adds support for ?. to pregenerated Dart Change Detectors. Closes angular#791
Adds support for ?. to pregenerated Dart Change Detectors. Closes angular#791
@vicb @mhevery Sorry for resurrecting this but I want to point out that the safe-navigation-operator and the "Elvis" operator are two different things. See Wikipedia , Groovy-Elvis, Groovy-Safe-Navigation. The Elvis operator is more akin to the Null coalescing operator with the difference that it tests for "truthy" instead of null for the lhs.
I think it is bad practice to take established terminology from other languages and change its meaning. What angular currently has is a safe-navigation-operator and not the Elvis operator. Calling it |
also related microsoft/TypeScript#16 but it seems that it's out of scope for typescript and needs to have an ES proposal |
I agree with leonard84, please rename this properly. All the different null'ish operators are confusing enough as they are. |
@wardbell Could we rename this operator per #791 (comment) in our docs? |
See also #6387 |
It's on my list, thanks |
Atom syntax highlighting breaks when interpolated expressions use the "safe navigation operator". Does anyone have an Atom syntax theme or package that supports the correct syntax highlighting for the "safe navigation operator" in html interpolated expressions? |
How to use it with object having special character keys viz. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…r#828) Fix angular#791 Build the render tree starting from the root node of the application. This fix is applicable only for v12+ apps that are using the latest debugging APIs.
Currently
a.b.c
will thrown an error in expression ifa
isnull
. To suppress null dereferencing we should supporta?.b.c
which would short circuit further evaluation ifa
is null. In this way we could have explicit notation which would show which values could benull
.Syntax to fallow dart proposal: https://github.com/gbracha/nullAwareOperators/blob/master/README.md
The text was updated successfully, but these errors were encountered: