feat(resource): static convention

[bigopon]

This PR is a merge of

• feat(ViewResources): auto register #610 : auto resource registration
• WIP feat(StaticViewStrategy): ability to statically analyze aurelia modules #606 : static view strategy

together with

• ability to specify aurelia resource type via static resource field, method on class, as a replacement for
  @customElement, @customAttribute, @valueConverter, @bindingBehavior, @viewEngineHooks calls

usage examples:

Declaring a custom element <normal-button/>, containerless, with a bindable type attribute

  class NormalButton {
    static resource () {
      return {
        name: 'normal-button',
        containerless: true,
        bindables: ['type']
      };
    }
    static view () {
      return '<template><button type.bind="type" >Normal button</button></template>';
    }

    // or jsx
    static view() {
      return <template>
        <button type$='type' onClick='shout()'>Normal button</button>
      </template>
    }
  }

no type specified = custom element

declaring a custom element '`

  class NotSoFancyButton {
    static resource () {
      return {
        name: 'fancy-button',
        bindables: ['type']
      };
    }
    static view () {
      return '<template><button type.bind="type" >Fancy button</button></template>';
    }

    // or jsx
    static view() {
      return <template>
        <button type$='type' onClick='shoutInFancyWay()'>Fancy button</button>
      </template>
    }
  }

declaring a custom attribute [date-picker]

class DatePicker {

  static inject () {
    return [Element];
  }

  static resource = {
    type: 'attribute',
    name: 'date-picker',
    bindables: [
      { name: 'date', defaultBindingMode: 'twoWay' }
    ]
  }

  constructor (element) {
    this.value = '';
    this.element = element;
  }

  attached () {
    this.element.type = 'date';
    this.element.addEventListener('input', this);
  }

  detached () {
    this.element.removeEventListener('input', this);
    this.element.type = 'text';
  }

  handleEvent (e) {
    this.value = this.element.value;
  }
}

Declaring an app custom element

export class App {
  // string mean it's a custom element, name equals to the string
  static resource = 'app';

  static view() {
    return {
      template: `<template>
        <form>
          <input date-picker.bind='date' />
          <fancy-button></fancy-button>
        </form>
      </template>`,
      dependencies: [FancyButton, DatePicker]
    }
  }
    
  // or jsx
  static view() {
    return <template>
      <form>
        <input datePicker$='date' />
        <fancy-button type$='type'></fancy-button>
      </form>
    </template>
  }
}

The static resource will be looked for before traditional convention, such as export name.

pros:

• helps devs write clean class, without any trace of aurelia, as demonstrated via above examples
• provides convention that easily works without transpiler, means later, it will be a simple <script src="aurelia.umd.js"></script> and plain javascript classes

cons

• traditional convention via loader not applicable. (though there is probably a way to support loader, with a requirement that everything must be an absolute url)
• uncertain what will come

todo:

• test
  ...

tobe discussed:

static resource signature

  export class ViewResources implements IViewResources {

    static convention(target: Function & { resource?: string | IResourceConfig | { (): string | IResourceConfig } })
    // ...
  }

  export interface IResourceConfig {
    type?: 'element' | 'attribute' | 'valueConverter' | 'bindingBehavior' | 'viewEngineHooks';
    name?: string;
    bindables?: Record<string, boolean | IBindablePropertyConfig>;
  }

Not sure if bindables should be an object with each property map to a bindable property, or an array of either string or a bindable property config with name in it. With array, name is required if it's not a string

static field / method name

resource was picked because it aligns with internal term of Aurelia, but doesn't look friendly on famework user side. Probably metadata could do a better job. Should it be metadata instead of resource ?

@EisenbergEffect @jdanyow @jods4

---

Update 1:

• Features map

1. This PR, for easy resources recognition convention, resources registration via classes, static view for local view resources dependencies declaration via classes
   1.1 global resources declaration via classes feat(config): ability to declare global resources by classes framework#858
   --- templating-router resources refactor(configure): register resources synchronously templating-router#73
   --- templating-resources perf(all): synchronous resources registration templating-resources#340
   1.2 Future pr, where plugin accepts a configure function, instead of module id, to be called in sequence when aurelia.start is called
2. Add new feature to composition engine: ability to compose if viewModel specified is a constructor feat(CompositionEngine): allow constructors #611
   2.1 Ability to use class as first parameter for Aurelia.setRoot feat(Aurelia): ability to define root with constructor framework#876
   2.2 Ability to use class / normal module for router WIP feat(RouterView): ability to use class as moduleId templating-router#75

[fkleuver]

I really like this idea!

• I think bindables (if multiple) should be an array. Having array/function convention on static properties is in line with how static inject works. And you could normalize a single object to an array for the occasional user that only wants to define a single one (often the case with attributes).

• Nothing wrong with the name resource imo. We've had a resources folder and .globalResources() call since day one, which always pertained to elements/attributes etc (exactly the thing you're addressing). Metadata on the other hand is a fairly abstract term to people who never worked directly with it (which is probably most people).

Just my 2c.

[zewa666]

@bigopon could you update above samples with one using JSX for the view?

[bigopon]

@zewa666 Sure, you can also play with it at https://stackblitz.com/edit/aurelia-static-convention?file=app.js

@fkleuver I think array gives better config API too, you can have a play in demo link above

[fkleuver]

@bigopon Will this only work when the properties are functions (seems that way right now)?

[EisenbergEffect]

@bigopon I'd love to support both static methods and properties if possible.

[bigopon]

No it can be either object or string or function resolving to one of those. Static method is what works today without transpiler so i used it

[3cp]

I can see the good will of supporting jsx as inline template, but the side effect is some unnecessary learning curve.

Now for users want to use inline template, they need to understand how jsx works, and also how to map its features to Aurelia. We don't need that learning curve.

Instead of:

  static view () {
    return <template>
      <input datePicker$='date' />
      <button type$="type" onClick='shout()'>Normal button</button>
      {'${message}'}
    </template>;
  }

We can just write:

  static view () {
    return `<template>
      <input date-picker="date">
      <button type="type" click.trigger="shout()">Normal button</button>
      \${message}
    </template>`;
  }

There are two minor issues around using es6 multiline string literal.

1. there is no syntax check for inline html from editor/transpiler.
2. need to use \${m} to bypass es6 string interpolation.

To make it better, but instead of using babel plugins syntax-jsx, transform-react-jsx, I suggest to develop new plugins syntax-jsau, transform-jsau to support inline html.

name jsau might be miss-leading, as this new format has no dependency of aurelia. It just tries to support inline html (real html) in js file.

This is actually technically simpler than trying to mock React.createElement.

• syntax-jsau is easy to implement than syntax-jsx, because we can use a DOM parser to parse html, I mean real html by standard, not jsx html.
• transform-jsau is really dumb, just wrap the contents in a js string, instead of parsing it into React.createE ....
• need some work on babel-eslint to support jsau syntax check.
• need more work on popular editors to support jsau syntax check.

So we can finally write:

  static view () {
    return <template>
      <input date-picker="date">
      <button type="type" click.trigger="shout()">Normal button</button>
      ${message}
    </template>;
  }

[zewa666]

@huochunpeng jsx was purely out of interest to see whether it's doable. It would also provide a nicer migration for existing React code. So I definitely don't envision to push users into JSX direction, more just to know that it kinda works

[3cp]

That makes sense. I can see from stackblitz jsx is implemented purely by a mock of React.createElement.

[bigopon]

A custom plugin to display html in jsx is awesome, but would be a lot of work I believe. I have no experience of that, maybe @eriklieben, who has been working on vscode extension, can give some comments. Personally I believe complex markup needs to be in their own well formatted file. So final usage would be a plugin to inline the template during build time (single file module / component etc), or devs explicitly import view in the view model file and use it. Though inline view with template literal is very good for demos / getting started. Also another way is to get it directly from index.html, as view works with HTMLTemplateElement too:

<!-- index.html -->

<body>
  <div id="app-main-template">
    // heavy markup prerender by server
  </div>
</body>

class App {
  static view() {
    let template = document.createElement('template');
    template.content.appendChild(document.querySelector('#app-main-template'));
    return template;
  }
  // ...
}

[bigopon]

@EisenbergEffect Tests for all new features added and ready for review.

[3cp]

So final usage would be a plugin to inline the template during build time

As long as not using webpack, this can be a small gulp plugin to find matching vinyl files (js and html) and merge them, at gulp stream flush stage.

[3cp]

Had a brief talk with @bigopon about sub-class support, glad to know it all works.

Here is some code example for reference.

class AnotherButton extends NormalButton {
  static resource() {
    let res = super.resource();
    res.name = 'another-button';
    return res;
  }

  shout() {
    super.shout();
    alert('Actually clicked another button :-)');
  }
}

[EisenbergEffect]

@bigopon Since things have changed a bit in the latest set of iterations, can you update this issue with the full list of PRs, the order they need to go in and any other relevant information? I can start with "ring 0" this week and we can release them all over the next 2-3 weeks, while we work on some documentation.

[bigopon]

@EisenbergEffect i have updated the PR with the road map how things go together

[EisenbergEffect]

Thanks!

[obedm503]

does this pr still work with decorators like #606 did?

[bigopon]

@obedm503 New check was added for ViewLocator to check static view property on the class, not much else. So there is no decorator. But it can be easily created:

export function view(config: string | IStaticViewConfig): any {
  return function deco(target: Function) {
    target.view = config;
  }
}

@EisenbergEffect should be provide a decorator ?

[obedm503]

It'd be nice to have. I prefer decorators to static properties

[fkleuver]

Probably good idea to include that decorator source code in the docs as well. It's nice to define your own decorators in larger apps as a way to apply common logic/conventions and stuff. More people should know about it.

[jods4]

In main codepath, name (here config) goes through _hyphenate.
Here it doesn't, is that on purpose?

[bigopon]

🐛 😱

[bigopon]

Rethinking about it, probably I will have to do it differently: if name was specified, use it and lower case it, otherwise hyphenate the class name. It's more consistent with current way how it works.

[jods4]

I agree. It's strange that we modify an explictely defined name.

Hypenate made perfect sense when we derived the element from class name.

[jods4]

Nit: No space after //, custome typo.

[jods4]

Could be moved inside previous if?
Same check twice in a row just looks weird.

[bigopon]

It's the way we check for custom element and custom attribute.

Try with convention first, if nothing matches > fallback to element

[jods4]

I understand what it does but it looks weird. Basically you do:

if (A) {}
if (A) {}

If the first A is false, the second has to be false as well and is redundant.
Nitpicking anyway, it works fine.

[jods4]

Nit: copy-pasted comment with same defects as previous one.

[jods4]

I think I get this based on previous discussions we had.
But it really needs some explanations and some tidy-up (notice for example Function[] is redundant in your return type).

If I get this right, you 100% need to explain that:

• it returns an array of promises rather than a promise for an array to easily support [() => import("x"), () => import("y")] pattern.
• that the Record<string, Function> is meant for lazy loading based on resource name, so the Function here is a factory unlike all other Function in that declaration, which are class ctor. OK, now I think it's for ES modules from () => import("x"), right?

[jods4]

We might want a factory returning a single resource, i.e. a function?
It might be a typo but it's not in the typedef above.

There are better use cases, but this: () => [ async () => (await import("x")).MyElement ].

[bigopon]

Yes, Record<string, any> is for import('x'). That said, I just realized it's doesn't handle the case where an export is not a function. Probably we should ignore any export value that is not a function.

[jods4]

Nit: no, thanks.
You did get the precedence of everything right, but that's too much trivia for real code.
I suggest at least reordering the checks so that there is no ambiguity in the ternary operators results:
isFunction ? deps() : isArray ? deps : deps || []
That's still a bit cryptic to me, but at least there's no ambiguity in results.
Note that it works because isArray simply returns false for anything that's not an array, including null, undefined.

[bigopon]

I think I have fixed this in the followup PR. Reading it again does feel terrible

[jods4]

So... I don't quite get it.
What's the point in supporting a record if we do nothing with key?
Is it so that future versions could lazy-load resources when their name is first encountered in a view?

That's a cool future feature, but then I think the best choice is to not accept a record until then.
Because lazy loading is a breaking change, so we don't want people to use this syntax until we actually implement the feature.

Even so, it's not implemented correctly. As lazy-loading implies a factory, a correct eager-loading implementation would be: dep[key]().

[jods4]

Oh wait, I think I get it now...
This is for the case () => import("y"), isn't it?
The record is actually an ES module?

Makes sense then.
I like how we try to find all resources in said module, rather than just the first one as in older Aurelia code 👍
No reliance on default, then.

Worthy of a short explanation or comment, though.

[bigopon]

Yes, It is also consistent with the way we currently load resources if it was string: every thing will be registered. And sure, docs for this work gonna be fun...

[EisenbergEffect]

@bigopon I recall you needed some resolution on this to move forward. Gitter is broken for me right now so I haven't been able to read your messages or respond to you ;( How can I help move this forward?

[bigopon]

@EisenbergEffect

Summary from @jods4 & @StrahilKazlachev 's review

Checking for a static method / field view could introduce breakages into existing app that use this name.

---

I think we may need to replace it with symbol, e.g Symbol.for('au-view') instead of normal name. So that folks who wish to use static can still use it, though less convenient than old name:

class MyElement {
  static [Symbol.for('au-view')] = '...'
  // or
  static [Symbol.for('au-view')]() {
    // ...
  }
}

Same thing may apply for resource. Besides that, there are few other issues related to convention that I will fix after we decide what to do with naming, like this #614 (comment)

[EisenbergEffect]

Ok, I thought that was the main issue. I'd prefer not to use a Symbol though. That makes for a messy api. One thought I had was to make this an opt-in framework feature. Can we have a setting somewhere, that only checks for the static fields if you have enabled it? That would allow us to keep the api we want while providing backward compat.

[bigopon]

That will work. Then how would you suggest for interop between static opt in and decorators ?

[EisenbergEffect]

If someone uses a decorator that would create a static prop, but they haven't enabled the feature, we should throw with a clear error message explaining that they need to opt into the new model and how to do that.

[bigopon]

There is probably no reason why using decorators only would not work, should throwing error would be unexpected ? Or you suggesting that we need a strong switch for mental model so folks can be aware of side effect of these static features ?

[EisenbergEffect]

I was thinking that you thought there would be clash between the two, so I was recommending a potential solution. I may not have understood your question then...

[bigopon]

Oh I see what you meant. Because decorators resolves to static field, that's why you suggest throwing an error. I was thinking decorator could generate some metadata, while static field ... declare its static bit and we make them interop-able. But we could do as you suggested, always resolve to static method / field and add a flag to opt it in.

[EisenbergEffect]

Yep :)

[jods4]

@EisenbergEffect I know I can't have it both ways: back-compat and easy use of new releases...

But still I'm not fan of the opt-in API. This way will become the way to go, especially with Webpack and co., but if it's behind a config it won't be obvious and easy to get started.

What if we use an inconvenient enough property, so that we're 90% sure it won't break anyone. I previously suggested:

class X {
  static ["au-view"]  = "<img>";
}

It's not that bad, but the presence of dash makes it highly unlikely to have been used by someone.

[jods4]

One more thing I just thought about: we need this enabled by default, somehow.

Here's why:

This has tremendous value because it would enable us to drop our Webpack plugin in new projects and support many bundlers. This is the main motivation behind this work.

To drop the Webpack plugin, we need our core libs to be updated to use this new static conventions.

If we update core libs to use this, it's not an opt-in anymore.

[bigopon]

I support what @jods4 suggested. Using a switch also gives plugin authors dilemmas if they wanna use this feature, it will be eventually on for the rest of the application if a plugin decides to turn it on. Unless it also accepts an additional static prop to turn it on and off per class

[obedm503]

But that's on the way to too much config. I honestly don't see a problem with using symbols or even Reflect.metadata as long as it's managed carefully

[fkleuver]

@obedm503 Main problem with symbols is they would require a lot more changes (and in some places plain additions, which can cause performance degradation). They aren't discoverable in the same way as normal properties. for .. in, Object.keys(), Object.getOwnPropertyNames and Object.hasOwnProperty don't reveal symbols.

However I don't see any issues with poor man's Reflect.metadata, which is to just put a __aurelia__ property on everything with Object.create(null). Would arguably make things even simpler and more performant than they are now. But I think that's a different discussion.

[obedm503]

Right, forgot about symbols not being enumerable

[EisenbergEffect]

Why don't we use $resourse and $view? That would avoid conflicts and enable a simpler syntax for usage. There's also precedent with this type of naming pattern in other frameworks. Additionally, I'm using a similar naming scheme in the vNext work.

[jods4]

I agree there's a precedent for $ variables being "special" or "reserved", it's quite unlikely someone has a static method or field starting with $ on a VM.

I would even say that it hints the fact that the field is "magical", which is a rather good thing. Conventions can sometimes be hard to understand for newcomers.

So if there's no better proposal... $view?

[fkleuver]

Agreed, $ is an unambiguous prefix. If it's also used in vNext then that will make the transition that much smoother.

[obedm503]

why not something super ugly and even less likely to be already used like __aurelia__view__ but when using it aurelia could export an enum to make usage easier

// some aurelia file
export enum AureliaMagicFields {
  view = '__aurelia__view__'
}

// user - app.ts
export class App {
  static [AureliaMagicFields.view] = []
}

I recommend this for a few reasons:

• it's very unlikely that this static property will be reassigned later in code, so the inconvenience of a computed key is not that bad. generally it will just be declared up front
• it adds strength to the api. if someone misspells $veiw it will probably take longer to debug than misspelling [AureliaMagicFields.veiw] because either the linter or their compiler will complain

I guess it doesn't even have to be an exported enum, just a constant string will do as well. also, if aurelia ever wanted to move to symbols for these kinds of magic/conventional fields, the breakage, from the users' perspective, wouldn't be as bad if they're already using computed fields

[jods4]

@obedm503
If we go with something based on an import, like :

import { view } from "aurelia-symbols";

export class App {
  static [view] = "";
}

Then I would argue there is absolutely no reason not to go with a symbol instead of a string.

[obedm503]

@jods4 I think it's best form every package to export their own constants instead of a single package with all of them. say this idea was accepted and user in other places, like the lifecycle methods

// import {viewConstants} from 'aurelia-templating';
enum viewConstants {
    view = '__au-view__', // or = Symbol('au-view')
    resource = '__au-resource__', // or = Symbol('au-resource')

    attached = '__au-attached__', // or = Symbol('au-attached')
    detached = '__au-detached__', // or = Symbol('au-detached')
}

// import {routerContants} from 'aurelia-router';
enum routerConstants {
    activate = '__au-activate__', // or = Symbol('au-activate')
    deactivate = '__au-deactivate__', // or = Symbol('au-deactivate')

    // previously `configureRouter`
    configure = '__au-configure__', // or = Symbol('au-configure')'
}

class Component {
    static [viewConstants.resource] = 'x-component';
    static [viewConstants.view] = '<template><button>Normal button</button></template>';

    handleClick = () => {}
    [viewConstants.attached]() {
        document.addEventListener('click', this.handleClick);
    }
    [viewConstants.detached]() {
        document.removeEventListener('click', this.handleClick);
    }
}

class Parent {
    router: any; // Router
    // previously `configureRouter`
    [routerConstants.configure](config, router) {
        this.router = router;
    }
}

class Page {
    static [viewConstants.view] = '<template><x-component></x-component></template>';

    [routerConstants.activate](params) {}
    [routerConstants.deactivate](params) {}
}

just for the record I'm not suggesting viewConstants or routerConstants as names, these are just for the example

@EisenbergEffect @bigopon what do you think of the idea of using contants (strings or symbols) for aurelia-magic/reserved methods and properties?

if the idea is green-lit I could work on a pr to use constants for static properties and functions. I'm still not totally sure about lifecycle methods.

[fkleuver]

I use symbols exclusively in a plugin where I wrap and intercept the configureRouter method of a user's view model. I think that's one of the few use cases where using a symbol is the only correct thing to do.

Most libraries use generated/long names when modifying globals or objects they do not own, such as html elements. Aurelia also does this in those cases.

When it's the user putting these properties on their own classes, why worry so much about potential conflicts? Users will have more stuff to learn about and more stuff to import. I feel like it's unnecessary bloat but maybe I'm missing something..

[obedm503]

As I said, when it comes to lifecycle methods I'm not sure it's necessary. But when it comes to something like static view or resource, there IS a potential for conflict since these are new reserved names. It's entirely possible that someone's using one of these names already and by introducing this feature their code will break.

The suggestion of using imported constants removes that possibility for breakage