-
Notifications
You must be signed in to change notification settings - Fork 18
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
Glimmer Component API #95
Comments
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 Thankfully, the way it's been re-written, it's almost at the point where it will work without |
The APIs are similar enough that it would probably work with both of them, I'd say go for it 👍 |
Awesome. I'll play with that idea once I've had a chance to get the performance optimizations that we want to do |
Any idea to "merge" in It's not good to have |
Since the native class RFC, this is native Ember behavior. It is also misleading in that, if you supply an import Component from '@ember/component';
import { argument } from '@ember-decorators/argument';
export default class ExampleComponent extends Component {
@argument('string')
arg = 'default';
didReceiveAttrs() {
super.didReceiveAttrs();
console.log(this.arg); // => undefined
}
} {{example-component arg=undefined}} There's been talk about a For Glimmer components, what we'd need is something like import Component from 'sparkles-component';
import { argument } from '@ember-decorators/argument';
export default class ExampleComponent extends Component {
args: {
arg: string;
};
@argument('string')
@readsWithDefault('args.arg')
arg = 'default';
didReceiveAttrs() {
super.didReceiveAttrs();
console.log(this.arg); // => 'default'
}
} Applying the decorators in that order would also eliminate the need for changes to |
I see your point, makes sense. Having Given your examples, I question the "position" of the export default class ExampleComponent extends Component {
@validation('foo', {type: 'string', required: true})
args: {
foo: string;
};
@readsWithDefault('args.foo')
foo: string = 'bar';
} 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('foo', {type: 'string', required: true})
export default class ExampleComponent extends Component {
@readsWithDefault('args.foo')
foo = 'bar';
} I also maybe played around with a different syntax for |
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 import { argument as type } from '@ember-decorators/argument'; If that feels better in the meantime! It might be worth considering re-naming this library before we hit |
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 With that in mind, the reasoning behind 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 My main concern is with finding a nice syntax for supplying default values to arguments. My codebases have been using the For me, the best ergonomics of the I'd also like to see |
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. |
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 |
@alexlafroscia Thanks for the response. Super bummed, was really hoping to have something to replicate React's |
Also, should probably add a note in the README about this. Especially if you're not planning on actively maintaining the package anymore. |
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: const types = {
foo: 'string',
bar: 'number',
baz: (value) => {
// custom validation logic
}
};
const defaults = {
foo: 'hello',
bar: 'world',
baz: () => [] // init an array
}
<template>
{{arg-types types}}
{{arg-defaults defaults}}
</template> This way we're minimizing the micro syntax required, and we have a solution that works in both TO and class components. |
That'd be awesome. Though I would prefer doing something like: const types = {
foo: argTypes.string,
bar: argTypes.number,
}; Again, like |
Would |
|
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 theargs
object on the instance. This means that this addon is incompatible with Glimmer Components.To get a feel for how they work, you can use
sparkles-component
.The question I want to solve is: How can we expose the same validation system to Glimmer Components?
The text was updated successfully, but these errors were encountered: