Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Should directives and formatters have to extract expressions ? #1050

Open
vicb opened this issue May 16, 2014 · 8 comments
Open

Should directives and formatters have to extract expressions ? #1050

vicb opened this issue May 16, 2014 · 8 comments
Assignees

Comments

@vicb
Copy link
Contributor

vicb commented May 16, 2014

Right now there is some specific code in html_extractor.dart to extract the expression from the html files at build time.

It seems some cases are missed, see #1023, #993 for examples.

What if the directives / formatters hold the code to extract that info from the html ?

I see several advantages:

  • factorize some code,
  • easier to forget to sync the transformer side with the current system,
  • allow user directives to have their own extraction scheme.

There production code size should not changed as the tree shaking process will remove the methods used only at build time.

Thoughts ?

@pavelgj
Copy link
Contributor

pavelgj commented May 16, 2014

Could you please elaborate on "directives / formatters hold the code to extract that info from the html"? What API do you envision?

@vicb
Copy link
Contributor Author

vicb commented May 19, 2014

I have to think more about the details but the basic idea would be:

  • a Directive or Formatter could implement the Extractable interface,
  • it would implement a extractExpressionFromNode method,
  • the method would return a list of expression (or null)

Some utils need to be exposed to those client code:

  • matchesNode(),
  • some parsing support ? (parsing with regexp could be tedious)
  • ...

@jbdeboer
Copy link
Contributor

+1, I like this general design.

It will also make our code flow wrt performance more managable (e.g. extractExpressionFromNode should not be accessible outside the compiler)

@vicb could you put together a quick estimate of the work involved with this change? If it isn't much, just send a PR :-)

@pavelgj
Copy link
Contributor

pavelgj commented May 23, 2014

While I can imagine how this can be useful I believe it's absolutely unnecessary for #1023 and #993.

#993 -- ng-style is simply missing exportExpressionAttrs: const['ng-style'] in the Decorator annotation. I could swear I added it there at some point... but I don't see it in the history. Maybe I'm confusing with ng-class

#1023 -- I think filters are just not handled by the expression extractor, which is a oversight and easily fixed. Filter syntax is well defined, no issues there.

Your proposal might be a good fit for directives like ng-repeat which have custom syntax, but we have few of those.

Also, expression extraction happens at compile time, statically... I'm having hard time imagining how you plan to execute the code in the directive/component at compile time... I guess that's why I asked for an example of the API.

@pavelgj
Copy link
Contributor

pavelgj commented May 23, 2014

I take back my comments about #1023. OrderBy filter falls into the same category as ng-repeat -- it requires special treatment by the expression extractor.

@vicb
Copy link
Contributor Author

vicb commented May 23, 2014

@pavelgj thanks the suggesting a fix for #993 I have just pushed a commit to the pre-submit queue.

My initial thinking is to get a DirectiveMap and a FormatterMap and to iterate other those for each Node.

I'll try to craft a proto next week to see:

  • if this is feasible at all,
  • what the dev effort would be.

@vicb
Copy link
Contributor Author

vicb commented May 23, 2014

@pavelgj I think I get what you said now: I can only get a DirectiveMap for the core directives, not the user defined ones.

But we parse all the directives / formatters anyway. So if extractExpressionFromNode is a static method then we should be ok (hopefully).

@justinfagnani
Copy link
Contributor

I just ran into a case where a directive needs to extract expressions from an unknown set of attributes.

One solution for running code at compile time is to let a custom extractor script file be declared in pubspec.yaml, then spin up an isolate to run it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants