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

Angular2 support #39

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Angular2 support #39

wants to merge 4 commits into from

Conversation

sintro
Copy link

@sintro sintro commented Dec 15, 2016

I am creating this PR, so it will be possible to improve new code and may be fix some limitations, noted in my README. There is only one file with changes, which should me merged in base repo - lib/parser.js

@denar90
Copy link

denar90 commented Dec 15, 2016

@sintro can you resolve merge conflicts (I think it's better revert changes in these files)?
@yury could you spend some time on looking into it. We are working on slm-brunch and wanna use your repo as dependency, but with ng-2 support. Can you make code review and point on places where improvements are needed?
We would really appreciate that. Thanks 😃

@yury
Copy link
Contributor

yury commented Dec 16, 2016

@denar90 it would be great if you wrote some tests. So I can jump in to hacking without breaking anything...

@sintro
Copy link
Author

sintro commented Dec 16, 2016

@yury, should I remove changes in README and package in PR, or you will just ignore them when resolve conflicts?

I also want to note, that I made this fork to creatу slm-brunch plugin, and I faced strange problem: I could not make html5 doctype, so every attributes which don't need value (disabled, etc) were compiled like disabled="". As I am using same compile mechanics for hash-attributes (like #item), it is rendering like #item="". I workarounded this with this function: https://github.com/sintro/slm-brunch/blob/master/lib/slm-brunch.coffee#L31 , which is search for engine with 'format' property and change it to "html" before compiling.

It will work fine in future, but in 'slm' template engine, if you want to use it ng2 support, you have to ensure html5 format is enabled. Thus, I can offer to make it default value for compiling, how do you think?

@sintro
Copy link
Author

sintro commented Dec 16, 2016

Also, do we need the dynamic code values for #hash attributes? I noted in README that I didn't implemented them (There is also no possibility of making code values for hash-attributes (like #localvar=alert('Hello world')). I think it is excessive functionality). I don't think that it is hard to do, and to do template engine more organic and intuitive, it should me implemented.

@sintro
Copy link
Author

sintro commented Dec 16, 2016

I added base tests for angular2 supports
I did't cover code values at all for angular constructions, and also I did't implement any negative tests.
There are some fails already for some cases:

  1. Square bracket angular contrustions (either [attribute]="something" or [(ngModel)]="model") in square bracket wrapper (watch at bindings in square wrapper (without dot in attribute))
  2. Dots in binded attributes (like [style.color]) (watch bindings in round wrapper (with dot in attribute) and square bracket directives (with dot in attribute))
  3. Square wrapper for angular contrustions (watch ref- syntax referencing a template reference variable (square wrapper), attribute selector notation of components (square brackets + inline content), etc)

@yury
Copy link
Contributor

yury commented Dec 16, 2016

@sintro thanks for tests. What do you think about slim angular support

I think we can port it. What do you think?

I want to be close to slim.

@sintro
Copy link
Author

sintro commented Dec 16, 2016

I've saw that discussions before I started to implement my version of Angular2 support for js-version of Slim.
Why I didn't decide to repeat their deceision:

  1. To make it possible to write Slim templates with angular2 constructions without {}, but with possibility of using *ngIf or similar directives, it was necessary to make changes to splat-attributes syntax (for example, change one * to double **) (as I understood from Angular 2 bindings slim-template/slim#683 (comment)). Looks like there is no similar feature (splat-attrs) in js-version of Slim, thus there is no problem with using astrix without wrappers.
  2. In my opinion, forcing of using of ng2 constructions only inside of any kind of wrappers is limitation anyway, so it could be great if there will be both ways of writing code: with wrappers (may be it will be more reliable way) or without them (may be with some limitations, but I dont think there will be ones)

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

Successfully merging this pull request may close these issues.

3 participants