Skip to content
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

RFC: Create attrTypes to describe the interface of a component #35

Conversation

jonathanKingston
Copy link
Contributor

...nent

Rendered

@jonathanKingston jonathanKingston changed the title Create allowed-component-properties to describe the interface of a compo... RFC: Create allowed-component-properties to describe the interface of a compo... Feb 21, 2015
@mmun mmun self-assigned this Mar 3, 2015
@mmun
Copy link
Member

mmun commented Mar 6, 2015

I like this. It can land after 2.0 and I'm interested to see a PR after glimmer lands.

@mmun
Copy link
Member

mmun commented Mar 6, 2015

@locks
Copy link
Contributor

locks commented Mar 7, 2015

Shouldn't it be allowedAttributes instead of allowedProperties?

@mmun
Copy link
Member

mmun commented Mar 7, 2015

Yes. Still not a fan of it though. A list of allowed attributes is not very expressive. I prefer React's approach which lets you specify further constraints.

@jonathanKingston
Copy link
Contributor Author

@mmun I wanted to put in types however I didn't want the change to be too big initially without there being an interest for it.

The other thing is I was considering using the Ember data attribute types for declaring the data types, what do you think. The only other alternative I can see would be using Ember.typeOf results.

I like the react way of doing things.

@locks I did have originally two separate interfaces, where by you could specify the inbound attributes and the outbound properties. I think the outbound component to the components template is more important however attributes would probably chime more into glimmer I suspect.

@jonathanKingston
Copy link
Contributor Author

@mmun an alternative to the two separate interfaces would be having the ability to specify an attribute/property as private/protected.

My only worry with this is it is getting complex enough that I would personally prefer the computed property work to be expressed within this data structure.
So the new getters and setters syntax would be incorporated, similar to:

  myThing: Ember.computed('other', {
    type: 'string',
    private: true,
    get: function () {...},
    set: function () {...},
  })

@mmun
Copy link
Member

mmun commented Mar 7, 2015

@jonathanKingston I don't think it should be expressed as a CP. I was thinking of putting it on attrTypes, not unlike React. Something like

import { attrType } from "ember-component";

return Ember.Component.extend({
  attrTypes: {
    checked: attrType({ boolean: true })
  }
});

@jonathanKingston
Copy link
Contributor Author

@mmun I think there are two use cases here if both are important is a different story.

Template usage:

<my-component thing="element" thing1="element" ></my-component>
{{!-- thing1 would be ignored here --}}

my-component.js

import Ember from 'ember';
var attr = Ember.Component.attr;
export default Ember.Component.extend({
  attributeTypes: {
    thing: attr('string')
  },
  thing2: 'my thing',
  exposedProperties: ['thing']
});

my-component.hbs

  {{thing}} {{!-- element --}}
  {{thing1}} {{!-- '' --}}
  {{thing2}} {{!-- '' --}}

The ability to accept attributes and what gets exposed are distinct, I don't thing exposedProperties would need types as they come from the JavaScript internals.
I think exposedProperties is a better name for that use. 'permittedAttributes', 'allowedAttributes' or 'attributeTypes' is a better name for inboud.

The only other alternative is the parity with ember data which would cater for both use cases.

@mixonic
Copy link
Sponsor Member

mixonic commented Mar 7, 2015

+1 for attrTypes

@jonathanKingston
Copy link
Contributor Author

Working on the change now.

@mmun
Copy link
Member

mmun commented Mar 13, 2015

@jonathanKingston might wanna wait until after glimmer lands because it will massive simplify how attributes are propagated through a template.

@jonathanKingston
Copy link
Contributor Author

@mmun I was just writing up the changes that were discussed here, no real effort being expended:

jonathanKingston@5ddcdac

I will look over the latest changes there and see how that impacts this.

@jonathanKingston jonathanKingston changed the title RFC: Create allowed-component-properties to describe the interface of a compo... RFC: Create attrTypes to describe the interface of a component Mar 18, 2015
@jonathanKingston
Copy link
Contributor Author

@mmun one other thing that I have been toying with is the fact that it is very hard to get multiple properties into a component, much like you can for the link-to helper.

This could be resolved with this issue in several ways, however here is the most in keeping that I can think of right now:

Add in a serializedArray type to deserialize the type into an array:

<component-name models=(serializeArray model model2) >Text</component-name>

The serializeArray would work similar to JSON.stringify([model, ...]) however implementation doesn't matter so long as the attr type matches.

In the component:

export default Ember.Component.extend({
  attrTypes: {
    'models': attr('serializedArray'),
  },
  url: function () {
    return 'me/' + models.join('/');
  }.property('models')
});

@jonathanKingston
Copy link
Contributor Author

I forgot to update this to mention that @mixonic resolved my last comment in:
https://github.com/emberjs/rfcs/blob/master/active/0000-improved-actions.md

An attribute type would still be required for this data type however.

@igorT
Copy link
Member

igorT commented May 24, 2015

👍 on more complex validators.
Those would allow us to make the attrTypes work seamlessly between Ember Data's attrs and relationships and component's attrTypes

@tomdale
Copy link
Member

tomdale commented Jun 7, 2015

We would like to explore using JavaScript typesystems (e.g. TypeScript, Flow) rather than adding our own ad-hoc component type checking at runtime. If we do end up doing runtime type checks, we would like to explore using decorator syntax rather than attrTypes.

To move this forward, we would like to see it implemented as an addon that can be vetted in the community before it is supported in core.

@tomdale tomdale closed this Jun 7, 2015
@jonathanKingston
Copy link
Contributor Author

@tomdale thank you for reviewing this also.

I wasn't sure if that would be a direction that would be adopted by ECMA so I assumed that would be a no-go. The decorators solution certainly seems a viable option certainly.

Is this the direction Ember Data is going to take also?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants