search

PolymerElements / app-localize-behavior

Polymer behaviour to help internationalize your application
48 stars 54 forks source link

refactor(app-localize-behavior): code style #119

Closed AliMD closed 6 years ago

AliMD commented 6 years ago

I'm sorry to change code style but I think its little old! I try to improve the style and clean the codes and improve readability I hope you interested and merge it. Thank you in advance

merlinnot commented 6 years ago

Don't you think it should look like

/**
 * `Polymer.AppLocalizeBehavior` ...

?

The asterisk in the second line should be indented with one additional space.

AliMD commented 6 years ago

@merlinnot Thank you, its fixed

notwaldorf commented 6 years ago

Closing, since unfortunately we can't merge any ES6 code -- this is a hybrid element, which means that it needs to work in Polymer 1 apps as it is. IE11 does not support ES6, which means that the changes you added would break all IE11 apps, since they would need some sort of transform step.

AliMD commented 6 years ago

@notwaldorf I'm using it in IE11 and hybrid mode and its work great because with it will be compiled to ES5 with polymer build I don't understand! all polymer element use ES6 and work in hybrid mode! Whats the difference in this element?

notwaldorf commented 6 years ago

The point it that you can use this element without transpiling it to ES5. The Polymer elements are actually not written with ES6 at all -- you won't find any classes, lets and const. They depend on which version of Polymer they're running with -- and if you're installing this element while depending on Polymer 1 (which is 100% written in ES5), then what you get is a pure ES5 codebase, that doesn't need to be compiled for IE11

AliMD commented 6 years ago

@notwaldorf I don't understand, all other polymer elements updated to es6! who use this element separate all others ?!

notwaldorf commented 6 years ago

No other Polymer Elements are updated to es6; all the elements made by the Polymer team (in the PolymerElements org) are in es5. Please show me one element that has es6 syntax?

AliMD commented 6 years ago

@notwaldorf Wow! you are absolutely right! Any plan to move to ES6? How can I contribute? 😋

And If I convert the codes to ES5 is it useful to merge?

notwaldorf commented 6 years ago

There are no plans to move to ES6 right now for these elements, since this would break backwards compatibility. As a result, you also can't convert this element to a pure Polymer 2.0 style element (since that is inherently ES6). You can write your elements in ES6 (and pure Polymer 2.0 though)

If there were any other changes that weren't these 2, then sure, we'd love to see them, though generally with how many elements we have to maintain, just refactoring PRs tend to be pretty low priority for us :(