search

wix-incubator / react-templates

Light weight templates for react
https://wix.github.io/react-templates
MIT License
2.82k stars 207 forks source link

better rt-scope syntax parsing #175

Closed nippur72 closed 7 years ago

nippur72 commented 8 years ago

Currently the syntax parsing for rt-scope is quite weak not allowing templates like the following:

<div rt-scope="'as fast as possible' as message"></div>
<div rt-scope="';' as delimiter"></div>

The problem lies in expressions containing " as " and ";" because those are also delimiters used to scan tokens in the rt-scope attribute.

I think that can be slightly improved by having a proper regex that looks for the substring " as id;" where id is a valid JavaScript identifier and ; is optional.

Hopefully that should lower the risk of keyword clashing.

idok commented 7 years ago

cherry picked

nippur72 commented 7 years ago

@idok can you please comment about the remaining features in my PR? will they be merged or can I consider them rejected? do you want me to isolate them so that they have no merge conflicts?

idok commented 7 years ago

@nippur72 first I want to apologise for the delay in reviewing your work. If I am not mistaken I have cherry picked all of your changes besides lodash helpers and create element alias which are still in review. Regarding create element alias, it can potentially introduce a naming conflict which will be difficult to make sense of.

nippur72 commented 7 years ago

@idok thanks. Regarding createElement, the rationale is saving bytes (e.g. h()) and being able to use other engines like preact. There's something similar in babel and now also in TypeScript (flag --jsxFactory) so I think it makes sense to have that in our templates too.