You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The Glimmer Component API works different to the old Ember Component API. Arguments are not placed directly on the Component instance, but are set as the args object on the instance. This means that this addon is incompatible with Glimmer Components.
That's a really good question! I'd love to try to tackle making it work there.
I don't know a ton about the difference between sparkles-component and the direct use of Glimmer components. If I got this package working with sparkles-component, would it be safe to assume it would also work with Glimmer components at that time?
Thankfully, the way it's been re-written, it's almost at the point where it will work without EmberObject and init, so I don't think it should be too bad to make this work!
Any idea to "merge" in @arg from sparkles-decorators to provide defaults and being type-checked?
Since arguments don't have a default, which was the idea for @arg.
It's not good to have @arg and @argument. I think it's also more of a question in which direction the framework is moving here.
@ember-decorators/argument has become a validation only library, that has no effect on semantics. The usage example showing a default value might be a bit misleading in that regard, since @argument does not provide any default behavior any more.
Since the native class RFC, this is native Ember behavior. It is also misleading in that, if you supply an undefined / null argument, it will actually override the default value. In the same vein, if an argument that was present gets updated to undefined / null, the default value will not be restored.
There's been talk about a @defaultIfUndefined / ...ifEmpty decorator or something with a similar name. See miguelcobain/ember-yeti-table#17 for a crude implementation that is missing some key aspects.
For Glimmer components, what we'd need is something like @readsWithDefault.
Applying the decorators in that order would also eliminate the need for changes to @argument's validation system, if I am not mistaken. However, it does not look really ergonomic.
I see your point, makes sense. Having @arg and @argument is weird. But your semantics of @readsWithDefault() kinda imply (at least to me) that the default is given as parameter to the decorator.... I rested a little on this statement and tried thinking about just @read, when then ... it makes sense to have @readsWithDefault - but I guess it was also only a quick-suggestion by you, tricky naming beast :D
Given your examples, I question the "position" of the @argument decorator in your examples. Since they are on a property and properties aren't necessary mapped to arguments. I would put that onto args (also mostly only TS users will have the args written in there, because of typing purpose). If it would stick to the args property, then perhaps @validation is semantically more correct:
Yeah, I think you bring up a really good point here @gossi -- the naming of this library has been on my mind too. Calling it @type or @validate might make more sense -- especially since a Validator is now, internally, something you could potentially extend yourself to validate whatever kind of logic you want. Thankfully, there's nothing stopping you from doing this right now:
It might be worth considering re-naming this library before we hit 1.0.0 to be more inline with the current behavior which, as @buschtoens mentioned, is now validation-only with no behavioral side effects besides validating values in non-production builds (which is different than the original behavior).
To give some context here, part of the reason I wanted to scope this library down to focus on arguments specifically was because we were basically beginning to reinvent TypeScript with the @type decorators. In theory you can use them to check types anywhere, not just arguments, but it is tricky and a bit error prone, and really not as elegant as TS. My thinking was that focusing on component arguments, whose types can't currently be enforced, strikes a nice balance here, giving us some nice validations without going overboard.
With that in mind, the reasoning behind @argument vs @type was that it implies that scope. @type sounds like something you can apply anywhere - and people definitely will.
That said, if y'all want to go that direction, go for it 😄 I don't have enough time to actively contribute as much to this lib and I don't want to block experimentation and such.
I agree with @pzuraq. Seeing as this library is restricted to specifically type checking arguments, it makes sense that it implements a decorator named @argument, rather than a more nebulous @type or @validate.
My main concern is with finding a nice syntax for supplying default values to arguments. My codebases have been using the @argument foo = 'defaultValue' syntax since the 0.1.0 release of @ember-decorators/arguments and I've had absolutely no problems with that syntax.
For me, the best ergonomics of the @readsWithDefault functionality proposed by @buschtoens is applying it implicitly via the @argument decorator. I know the vision for this addon is to be validation-only, but this seems like the cleanest solution. Unless there's a good reason why that can't happen?
I'd also like to see @argument without any types passed as a lazy shorthand for 'any' - or at least some config to be able to allow this. This is simply for speed of development purposes, so I'm not bumping into an assertion because I haven't imported and assigned the right type for the argument when I'm just starting to sketch out the component's API.
From me, no, and I don’t think that @pzuraq is working on this package anymore either.
It’s not really clear how it would work, since Glimmer components access their arguments “directly” (rather than as a mix of external and internal state). Thinking about the this.args.FOO or @FOO syntax, there’s really no place to inject the logic around validating the values (unlike the Ember component API, where we could use a decorator to wrap the property and validate in a setter).
Agreed that we should definitely add a disclaimer about this.
@evanb2 re: something like propTypes, I think we do actually have some possible paths forward. Part of the issue is we really need to figure out default props and imports, as I think that the design of both of these would influence the design of argTypes, but currently I'm thinking of something like:
{{arg-defaults}} would need to set the actual value of the argument, both in the template and component js. I’m pretty sure it would require at the least a custom component manager, possibly would need to be built in.
Activity
alexlafroscia commentedon Jan 26, 2019
That's a really good question! I'd love to try to tackle making it work there.
I don't know a ton about the difference between
sparkles-componentand the direct use of Glimmer components. If I got this package working withsparkles-component, would it be safe to assume it would also work with Glimmer components at that time?Thankfully, the way it's been re-written, it's almost at the point where it will work without
EmberObjectandinit, so I don't think it should be too bad to make this work!pzuraq commentedon Jan 28, 2019
The APIs are similar enough that it would probably work with both of them, I'd say go for it 👍
alexlafroscia commentedon Jan 28, 2019
Awesome. I'll play with that idea once I've had a chance to get the performance optimizations that we want to do
gossi commentedon Jan 30, 2019
Any idea to "merge" in
@argfrom sparkles-decorators to provide defaults and being type-checked?Since arguments don't have a default, which was the idea for
@arg.It's not good to have
@argand@argument. I think it's also more of a question in which direction the framework is moving here.buschtoens commentedon Jan 30, 2019
@ember-decorators/argumenthas become a validation only library, that has no effect on semantics. The usage example showing a default value might be a bit misleading in that regard, since@argumentdoes not provide any default behavior any more.Since the native class RFC, this is native Ember behavior. It is also misleading in that, if you supply an
undefined/nullargument, it will actually override the default value. In the same vein, if an argument that was present gets updated toundefined/null, the default value will not be restored.There's been talk about a
@defaultIfUndefined/...ifEmptydecorator or something with a similar name. See miguelcobain/ember-yeti-table#17 for a crude implementation that is missing some key aspects.For Glimmer components, what we'd need is something like
@readsWithDefault.Applying the decorators in that order would also eliminate the need for changes to
@argument's validation system, if I am not mistaken. However, it does not look really ergonomic.gossi commentedon Jan 30, 2019
I see your point, makes sense. Having
@argand@argumentis weird. But your semantics of@readsWithDefault()kinda imply (at least to me) that the default is given as parameter to the decorator.... I rested a little on this statement and tried thinking about just@read, when then ... it makes sense to have@readsWithDefault- but I guess it was also only a quick-suggestion by you, tricky naming beast :DGiven your examples, I question the "position" of the
@argumentdecorator in your examples. Since they are on a property and properties aren't necessary mapped to arguments. I would put that ontoargs(also mostly only TS users will have the args written in there, because of typing purpose). If it would stick to theargsproperty, then perhaps@validationis semantically more correct:For JS only users, it wouldn't make sense to have it there, then I would say class level is my next bet (... and going back to
@argument):I also maybe played around with a different syntax for
@argument/@validation;) I think after all my comment is just loud brainstorming... not more.alexlafroscia commentedon Jan 30, 2019
Yeah, I think you bring up a really good point here @gossi -- the naming of this library has been on my mind too. Calling it
@typeor@validatemight make more sense -- especially since aValidatoris now, internally, something you could potentially extend yourself to validate whatever kind of logic you want. Thankfully, there's nothing stopping you from doing this right now:If that feels better in the meantime!
It might be worth considering re-naming this library before we hit
1.0.0to be more inline with the current behavior which, as @buschtoens mentioned, is now validation-only with no behavioral side effects besides validating values in non-production builds (which is different than the original behavior).pzuraq commentedon Jan 31, 2019
To give some context here, part of the reason I wanted to scope this library down to focus on arguments specifically was because we were basically beginning to reinvent TypeScript with the
@typedecorators. In theory you can use them to check types anywhere, not just arguments, but it is tricky and a bit error prone, and really not as elegant as TS. My thinking was that focusing on component arguments, whose types can't currently be enforced, strikes a nice balance here, giving us some nice validations without going overboard.With that in mind, the reasoning behind
@argumentvs@typewas that it implies that scope.@typesounds like something you can apply anywhere - and people definitely will.That said, if y'all want to go that direction, go for it 😄 I don't have enough time to actively contribute as much to this lib and I don't want to block experimentation and such.
willviles commentedon Feb 11, 2019
I agree with @pzuraq. Seeing as this library is restricted to specifically type checking arguments, it makes sense that it implements a decorator named
@argument, rather than a more nebulous@typeor@validate.My main concern is with finding a nice syntax for supplying default values to arguments. My codebases have been using the
@argument foo = 'defaultValue'syntax since the 0.1.0 release of @ember-decorators/arguments and I've had absolutely no problems with that syntax.For me, the best ergonomics of the
@readsWithDefaultfunctionality proposed by @buschtoens is applying it implicitly via the@argumentdecorator. I know the vision for this addon is to be validation-only, but this seems like the cleanest solution. Unless there's a good reason why that can't happen?I'd also like to see
@argumentwithout any types passed as a lazy shorthand for'any'- or at least some config to be able to allow this. This is simply for speed of development purposes, so I'm not bumping into an assertion because I haven't imported and assigned the right type for the argument when I'm just starting to sketch out the component's API.evanb2 commentedon Jan 22, 2020
Is there any plan to work on this enhancement now that Octane has been released?
This RFC had some great discussion but it sounds like any progress on this is a long ways out. Would love to see a solution here in the mean time.
alexlafroscia commentedon Jan 26, 2020
From me, no, and I don’t think that @pzuraq is working on this package anymore either.
It’s not really clear how it would work, since Glimmer components access their arguments “directly” (rather than as a mix of external and internal state). Thinking about the
this.args.FOOor@FOOsyntax, there’s really no place to inject the logic around validating the values (unlike the Ember component API, where we could use a decorator to wrap the property and validate in a setter).evanb2 commentedon Jan 27, 2020
@alexlafroscia Thanks for the response. Super bummed, was really hoping to have something to replicate React's
propTypesin the Ember-land.evanb2 commentedon Jan 27, 2020
Also, should probably add a note in the README about this. Especially if you're not planning on actively maintaining the package anymore.
pzuraq commentedon Jan 28, 2020
Agreed that we should definitely add a disclaimer about this.
@evanb2 re: something like propTypes, I think we do actually have some possible paths forward. Part of the issue is we really need to figure out default props and imports, as I think that the design of both of these would influence the design of argTypes, but currently I'm thinking of something like:
This way we're minimizing the micro syntax required, and we have a solution that works in both TO and class components.
evanb2 commentedon Jan 29, 2020
That'd be awesome. Though I would prefer doing something like:
Again, like
propTypes. But I also just really dislike the idea of using strings to define the types. Just my 2 cents.buschtoens commentedon Jan 29, 2020
Would
{{arg-defaults}}actually set default values for@fooor a differently named local scope variable?pzuraq commentedon Jan 29, 2020
{{arg-defaults}}would need to set the actual value of the argument, both in the template and component js. I’m pretty sure it would require at the least a custom component manager, possibly would need to be built in.