aurelia icon indicating copy to clipboard operation
aurelia copied to clipboard

Static analysis on router goto attribute

Open 3cp opened this issue 4 years ago • 9 comments

🙋 Feature Request

Static analysis on html template for router goto attribute <a goto="a-route-module"> to automatically bring in dependency on "a-route-module".

🤔 Expected Behavior

I noticed imports are required to wire the dependencies.

<import from="./welcome.html"></import>
<import from="./about.html"></import>
<import from="./books"></import>
<import from="./book-details"></import>

<nav>
    <a goto="welcome">Welcome</a>
    <a goto="books">Books</a>
    <a goto="about">About</a>
</nav>

But we could statically analyse the goto attribute to remove those import tags. So this can work:

<nav>
    <a goto="welcome">Welcome</a>
    <a goto="books">Books</a>
    <a goto="about">About</a>
</nav>

😯 Current Behavior

Currently user has to manually write import tags.

💁 Possible Solution

~~@aurelia/plugin-conventions can preprocess the goto attribute, and add them to dependencies array.~~ @fkleuver suggested AOT is the better place to implement this, as it can perform optimisation across group of files.

Note dynamic binding is not statically analysable, so user still needs explicit imports for possible route modules. For instance <a goto.bind="pickedRoute">.

🔦 Context

💻 Examples

3cp avatar Mar 18 '20 21:03 3cp

Plus support for

<au-viewport default="default-route" fallback="fallback-route"></au-view-port>
<div as-element="au-viewport" default="default-route" fallback="fallback-route"></div>

3cp avatar Mar 30 '20 01:03 3cp

How to deal with html-only component?

<import from="./about.html"></import>
<a goto="about">About</a>

Probably need to check existence of file about.js/ts/html/md/... to find out possible import.

3cp avatar Mar 30 '20 01:03 3cp

When using component names/strings in a goto it's the component with the au-viewport that needs to know how to resolve the component name, not the component with the goto. We've talked about also making it possible to resolve the component name in the context of the component with the goto, but it's currently not that way. So right now, the only time this would work is when the au-viewport component is the same or a child of the goto component. This will probably be quite common, but the cases where it isn't so might be a bit confusing.

I've thought about using some kind of static analysis to remove the need to use import/dependencies for basic cases, but didn't spend enough time on it to come up with a good enough convention for when the components are in different folders. Any suggestions?

jwx avatar Mar 30 '20 13:03 jwx

I got your point

my-app.html

<!-- routes must be imported here, not in my-nav.html -->
<import from="./welcome"></import>
<import from="./about.html"></import>
<import from="./missing"></import>

<import from="./my-nav.html"></import>
<my-nav></my-nav>

<au-viewport default="welcome" fallback="missing"></au-viewport>

my-nav.html

<nav>
  <a goto="welcome">Welcome</a>
  <a goto="about">About</a>
</nav>

I have no good suggestions. As user may hide gotos in deep child views, there is no way for static-analyzer to know where to inject deps.

If there is nothing we can do in conventions preprocessing, I will close this issue. Just let others to have a second look for a while.

3cp avatar Mar 30 '20 20:03 3cp

I like the idea. This is another place where our conventions would shine.

But if we can't make it work in all cases, it's probably going to add more confusion than developer happiness.

mobilemancer avatar Apr 06 '20 06:04 mobilemancer

Conventions feel like the wrong tool for the job here. They're there mainly for 2 reasons:

  • Productivity (less time spent writing code)
  • Clean code (less glue code bloating your business logic)

In this case, I'd want the IDE to offer autocomplete (based on all components living in the project), and automatically add the import when you pick one from the dropdown. Just like how VS Code does when you type new X.

This addresses:

  • manually having to write imports
  • not knowing which options are available when you type goto=
  • not knowing whether you did it correctly until you get a runtime error

Conventions would only address the first of these points and frankly I think the latter 2 are just as important, if not more important.

As for the "clean code" aspect of conventions, when it comes to imports I strongly feel it would do more harm than good to hide imports unless there is a tight integration between conventions and the vs code plugin that allows you to, for example, "goto definition" on the value of a goto attribute. For me the ability to "goto" from a component to its references is the cornerstone of long term maintainability of any project.

In summary, yes, I think we need this static analysis (and we can do it via AOT), but no, I don't think this belongs in the chapter of conventions.

fkleuver avatar Apr 06 '20 15:04 fkleuver

Thx! Let's close this.

3cp avatar Apr 06 '20 22:04 3cp

Why close? Loosely speaking, the initial phrase in your feature request:

Static analysis on html template for router goto attribute to automatically bring in dependency on "a-route-module".

Still holds true. Only the solution I propose (via IDE) is different from the one initially proposed here (via conventions)

fkleuver avatar Apr 07 '20 17:04 fkleuver

😆

fkleuver avatar Apr 07 '20 22:04 fkleuver