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

Add conversion in attribute format #16

Closed
benzen opened this issue Jan 20, 2018 · 8 comments
Closed

Add conversion in attribute format #16

benzen opened this issue Jan 20, 2018 · 8 comments

Comments

@benzen
Copy link
Contributor

benzen commented Jan 20, 2018

Here is an exemple in which it's unclear how things are transformed.

<template data-tagname="my-app">
    <app-entry my-attr="{{myAttr}}">
</template>
<template data-tagname="app-entry">
    {{my-attr}}
</template>
{
    "myAttr": "def"
}

This will render def.
As explained previously it's required by html (#15 (comment)).

It seams pretty natural to me to use lower-dashed-case for attribute (like normal attribute).
What is really weird to me is the fact that my data will change it's shape ().

What if the compiler did the job of converting back and forth the attributes and value path in data depending of the context ?

@caolan
Copy link
Owner

caolan commented Jan 20, 2018

Yes, it is a little strange to use variable names with a '-' in them in JSON data. I don't think we should automatically map variable names to a more JavaScript style though, partly because I don't see an accepted standard for the formatting of JavaScript variable names.

I like to think of each component as a function which accepts named parameters. Inside this component only the named parameters are available. It's more like introducing a new scope than changing the shape of the data.

That said, I'm open to discussing any other suggestions you have. Now is the time to make syntax changes if necessary (before there are too many server-side implementations).

@benzen
Copy link
Contributor Author

benzen commented Jan 20, 2018

I'm sorry, I wasn't clear.
What i meant was that we keep the data value/path as they are expressed.
Only make them standard compilient when it's time to output one of thoses to the Dom (in an attribute).

@caolan
Copy link
Owner

caolan commented Jan 21, 2018

I'm still not sure I follow. Can you provide me an example of this transformation?

@benzen
Copy link
Contributor Author

benzen commented Jan 22, 2018

Here is what i'd like to write:

<template data-tagname="app-todo">
  <my-other-component appTitle="{{upperTitle}}"></my-other-component>
</template>
<template data-tagname="my-other-component">
  <h1>{{appTitle}}</h1>
</template>

Here is the data

{
  upperTitle: "badAssTitle"
}

And here is what would be outputed in the dom, when the template app-todo is called.

<app-todo>
   <my-other-component app-title="badAssTitle">
    <h1>
     badAssTitle
    </h1>
  </my-other-component>
</app-todo>

Is this even possible?

@caolan
Copy link
Owner

caolan commented Jan 22, 2018

It might be possible for some server-side implementations, depending on what HTML parser they use. However, the JavaScript compiler uses the DOM, and that will only provide us with lowercase attribute names.

I think we should avoid mapping names internally as it complicates the compiler. However, if a server-side implementation can detect uppercase attribute names passed to components, it should lowercase them (and provide a warning).

We should add a test to magery-tests which checks for conversion of attributes to lowercase.

@caolan
Copy link
Owner

caolan commented Jan 22, 2018

I think the only other sensible option is to change the syntax for providing arguments to components, but the current attribute system is convenient and clean-looking, even if it in reality has a mismatch between acceptable HTML attribute names and acceptable JSON property names.

@benzen
Copy link
Contributor Author

benzen commented Jan 22, 2018

If the documentation explain why I think it s totally acceptable to Recommend the usage of dashed name in attributes.
React kind of forced every body to use camel case for everything.

@caolan
Copy link
Owner

caolan commented Jan 22, 2018

Closing in favour of #21 - marked as a bug as missing/incorrect documentation is a bug in my opinion

@caolan caolan closed this as completed Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants