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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] Remove safe-member access in expression #1808
Comments
Agree that this should be opt-in for some transition period. For example it could be completely removed in 2.1 minor release. |
@bigopon - Can we just grant access to users to do anything on the output of an expression and leave it to just call that func? bindingOutputExpression: (o:unknown) => o ?? '' |
I could see the benefits here. Mainly I think that you would be able to catch errors in templates sooner rather than later by realizing a certain dom element is not displayed properly. On the other hand, so much data is loaded async and the template doesn't have access to it right away. And many bindings occur on deeply nested properties all of which would require the new.? Syntax to move forward. It would be a little painful to have to type the new syntax in html templates vs just a period. Personally I think I prefer the v1 ease of use of not having to worry about undefined props to build templates. Many engines follow this format (handlebars, svelte, vuejs) so it could be seen as a con by future adoptors of Aurelia. It would probably be ideal to have an opt in to the old style so moving large applications is not so difficult |
As @roemhildtg mentioned, most template engines out there follow the current syntax, so it would just be a huge entry barrier for new users and will be very difficult to migrate for old users. As for the potential use case of catching errors in templates that arise due to deeply nested properties, I don't believe it's the responsibility of the template to handle data errors. The template should just render the data it is provided with, if it's valid, else not. The onus of validating the data that is provided to the template should be put on the view model. Personally, I'd not prefer the new syntax in the template. It just makes the template more complicated and harder to deal with, and I personally cannot see many benefits of migrating away from the current standard in the ecosystem, unless I am missing an obvious advantage of doing so (which I'd love to know) |
Strictness is very important thus should be always ON for new apps. It highly improves application behavior expectedness and reduces amount of non-obvious bugs. So it is concidered as a good practice, but of course it may be opt-out. |
Thanks @brandonseydel . We can, but that'll be a separate feature/RFC. Also I think it'll be enabling ad-hoc convention per app, or maybe even per section of an app. Unless you imply something else with the suggestion @brandonseydel. What we need to have decided is whether the property access |
Yes, I think I agree with this argument, nicely put 馃憤 . |
I thought of follow up questions.
One additional thought, if the user does opt in to not using the errors, could the template engine log a warning for undefined properties? I've seen this behavior in CanJS and it is quite useful. |
Very good idea! Not just for undefined, but warnings should express the same as TypeError's do, just without raising exception. @bigopon ? |
I do agree with this, but also I don't want to break the current method used by most other template engines. Why change something that is not broken?
This is the only thing I have a problem with new syntax, what would happen after throwing the error? Will the component render or will it just break? I am fine with throwing error in development and break the component but I don't want to break my page in production just because the component couldn't get data in time or got wrong data for whatever reason, a blank component with no content is better than no component and broken layout.
This is great! I think logging a warning is way better than throwing an error, at least in the initial versions and later convert it to error while adding a flag to revert to warning. tldr; Throwing error and breaking the component(with new syntax) during development is fine but retaining old behaviour is better for production. |
Yeah that'd be simple to add some warnings if applications decided to use the legacy behavior.
The template compiler is not capable of this in its current form since it'll need to evaluate properties on the view model to do this. We'll leave this job to the extension. |
Though this might be because
Having relied very often on the viewmodels just ignoring missing properties I think this should definitely be configurable (whether opt-in or opt-out) to ease migrations. |
馃挰 RFC
This RFC aims to align to standard JS how Aurelia template expressions evaluates.
馃敠 Context
At the moment, Aurelia template expressions are evaluated in safe-member access mode. This is the result of Aurelia being developed before the language has a mean to express optional access. For example, the following template will not throw:
even if the component
details
isnull
/undefined
. When evaluate propertyname
onnull
/undefined
, the result will beundefined
. This simplifies mental model dealing with template, as component can just cleanup (setting property values tonull
/undefined
) without having to worry/adding noises to the template (?.
/?.[]
/?.()
). Though this is not how normal JS behaves: there should have been aTypeError
thrown.With optional syntax fully supported in the template expression, we can consider ditching this old behavior and align fully with JS. This'll introduce complication for applications migrating from v1. As for applications that rely on this safe-access behavior to avoid breakage, expressions will need to start getting tweaked to use optional syntaxes. This could be a big task for some.
Another way to move forward is to allow both behavior exist at the same time and make the old behavior opt-in via a boolean, this could result in slightly more code but will be a big help for transitioning applications from v1.
The text was updated successfully, but these errors were encountered: