Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

RFC: Using statics, top-level members, enums in a template #374

Closed
matanlurey opened this issue May 12, 2017 · 20 comments
Closed

RFC: Using statics, top-level members, enums in a template #374

matanlurey opened this issue May 12, 2017 · 20 comments
Assignees
Milestone

Comments

@matanlurey
Copy link
Contributor

This is potentially planned for 4.0.0, pending feedback and experimentation.

tl;dr: We'd like users to be able to refer to:

  • Enums
  • Top-level fields and functions
  • Static members and functions of a class

... within their template (HTML).

Configuration

We'd need a way to tell the template what symbols to expect. There are several (unsatisfying) ways to solve this when it doesn't belong to the current library - for example, import with an unused import, or export. We're planning on settling on an exports: list on @Component:

import 'package:angular2/angular2.dart';

import 'some_library.dart' show topLevelField;
import 'another_library.dart' show SomeEnum;

@Component(
  selector: 'comp',

  // List symbols that will be accessible n
  exports: const [
    topLevelField,
    SomeEnum,
  ],
)
class Comp {
  @Input()
  SomeEnum value;
}
<div *ngIf="value == SomeEnum.status">You are a silly monkey!</div>
<button>{{topLevelField.toUpperCase()}}</button>

Examples

Top-level fields and functions

final okButtonTitle = "OK";

@Component(...)
class Comp {}
<material-button [title]="okButtonTitle"></material-button>

Enums

enum Status {
  loading,
  done,
  errored,
}
<div *ngIf="status == Status.loading">Loading...</div>
<div *ngIf="status == Status.done">Done!</div>
<div *ngIf="status == Status.errored">Error :(</div>

Static members and functions of a class

import 'package:intl/intl.dart';

class IntlMessages {
  static final hello = Intl.message('hello');
  static final cancel = Intl.message('cancel');
}
<button>{{IntlMessages.hello}}</button>
<button>{{IntlMessages.cancel}}</button>

Considerations

  • It is not clear whether your own component's statics will be exported "by default"
  • We should be able to support prefixed imports

/cc @MichaelRFairhurst, for considerations for the angular_analyzer_plugin.

@jonahwilliams
Copy link
Contributor

jonahwilliams commented May 12, 2017

What happens if exported symbols collide with either fields/methods on the controller class or each other? (Granted the analyzer plugin could probably help with this)

EDIT:

i.e. do you prefer exported, or prefer the specific kinds of exports.

And if there are collisions, is this a warning or error built into angular?

@matanlurey
Copy link
Contributor Author

I think we'd prioritize the instance member.

You could always use as to avoid this, but I do agree there is a chance of confusion.

@alan-knight
Copy link

I note that using final variables for Intl messages is not a good idea, because it introduces a race condition between locale initialization and variable initialization.

@matanlurey
Copy link
Contributor Author

@alan-knight:

I note that using final variables for Intl messages is not a good idea, because it introduces a race condition between locale initialization and variable initialization.

I think the assumption we make here is that you are configuring your locale prior to bootstrapping Angular. What about in that case?

@chalin
Copy link
Collaborator

chalin commented May 12, 2017

It will be great to have access to Enums, etc! A few comments.

(1) As for shadowing, I'd suggest that we follow whatever the rules are in Dart.

(2) Just to clarify, shouldn't exports be a List<String>? I.e., so we'd have:

  exports: const [
   'topLevelField',
   'SomeEnum',
  ],

(3) Finally, you give

final okButtonTitle = "OK";

as an example. Would you expect that to work if it were declared const instead?

cc @kwalrath

@jakemac53
Copy link
Contributor

jakemac53 commented May 12, 2017

(2) Just to clarify, shouldn't exports be a List<String>? I.e., so we'd have:

The exports list is designed to have actual references to const objects so that the compiler can resolve them at build time and know what imports to add to the generated template (as well as optimize based on their types). This also means you don't need to ignore any unused imports in your file or anything like that.

It does have the downside of only allowing you to export const objects to the template, but that still supports all the major use cases I think?

@matanlurey
Copy link
Contributor Author

matanlurey commented May 12, 2017

@chalin:

It will be great to have access to Enums, etc! A few comments.

(1) As for shadowing, I'd suggest that we follow whatever the rules are in Dart.

+1

That does bring up a different point, should statics of the class itself be visible, i.e. without including in exports: [ ... ] just following default shadowing?

class Comp {
  static const okButtonTitle = 'OK';

  static String formatObject(object) => ...
}
<button>{{okButtonTitle}}</button>
<button>{{formatObject('...')}}</button>

(2) Just to clarify, shouldn't exports be a List<String>? I.e., so we'd have:

  exports: const [
   'topLevelField',
   'SomeEnum',
  ],

As @jakemac53 mentioned, we want it to be actual references to the objects you will be accessing. Similar, to say, the directives: [ ... ] list.

(3) Finally, you give

final okButtonTitle = "OK";

as an example. Would you expect that to work if it were declared const instead?

Yup, definitely!

cc @kwalrath

@MichaelRFairhurst
Copy link
Contributor

MichaelRFairhurst commented May 12, 2017

I don't think the exports field would be necessary. It should be possible to analyze the expressions to find what needs to be imported.

If you think about it, that's essentially what the exports list is, whether its a list of identifiers (ie [foo, bar]) or a list of strings (['foo', 'bar']). Its a list of strings that the analyzer needs to resolve, from within a dart expression, to a list of import URIs. The only real difference between reading an exports array, and analyzing the template, is that the things that require special imports aren't neatly separated from the things that don't need them, but I don't think that's an issue.

I mean, you could maybe even just get around the whole problem if you assume all imports are used, and just copy all of them over into the generated template.

The bigger problem here is privates:

const _pi = 3.1415
...
exports: [_pi]

I think by not having an exports directive, the behavior here is quite clear: you can't use _pi inside your template because its private. By offering an exports field it makes it seem like maybe just maybe you could "lift" it to being public. Though regarding comments I've made earlier about having annotation data statically available at runtime, if that were added to dart, you could export _pi like this.

Alternatively, we could allow part template.g.dart in which case private classes, enums, methods, etc would all be available to the template.

Also:

any reason not to allow all things dart not currently allowed in templates?

  • await
  • constructors
  • anonymous functions
  • cascades
  • generics
  • interpolation
  • assignment
  • '??'
  • this/super
  • throw/retthrow
  • named arguments
  • is checks
  • as casts

?

@natebosch
Copy link
Contributor

There are a couple potential issues that I think would require a deep investment into the analyzer to resolve fully.

  1. Imports may be used only for the template and would show up as analyzer hints.
  2. An identifier could be ambiguous if it is present in multiple imports and unused in the Dart code.

The same reason we have the directives field is why we want exports. It can be a smoother experience to have the developer tell us exactly what they mean than try to figure it out and have confusing failure modes when we can't.

@matanlurey
Copy link
Contributor Author

matanlurey commented May 12, 2017

@MichaelRFairhurst:

I don't think the exports field would be necessary. It should be possible to analyze the expressions to find what needs to be imported.

How so? If you see [title]="format(Message.bar)" and your component looks like this:

import 'package:angular2/angular2.dart';

@Component()
class Comp {}

There is no clear way to find either format or Message. We considered alternatives, such as:

import 'package:angular2/angular2.dart';

// ignore: unused_import
import 'format.dart';
// ignore: unused_import
import 'messages.dart';

class Comp {}

or

export 'format.dart';
export 'messages.dart';

Both have obvious immediate downsides.

If you think about it, that's essentially what the exports list is, whether its a list of identifiers (ie [foo, bar]) or a list of strings (['foo', 'bar']). Its a list of strings that the analyzer needs to resolve, from within a dart expression, to a list of import URIs. The only real difference between reading an exports array, and analyzing the template, is that the things that require special imports aren't neatly separated from the things that don't need them, but I don't think that's an issue.

I mean, you could maybe even just get around the whole problem if you assume all imports are used, and just copy all of them over into the generated template.

We do that today, we actually don't want to anymore - we want to generate a Dart file that passes analysis and looks human readable (mostly). The only way we could truly due this is use part files, but that's a much harder transition.

The bigger problem here is privates:

const _pi = 3.1415
...
exports: [_pi]

I think by not having an exports directive, the behavior here is quite clear: you can't use _pi inside your template because its private.

This is already a problem: You can't use privates in directives, or providers, for example. It's unfortunate but the reality is our users are already used to this.

By offering an exports field it makes it seem like maybe just maybe you could "lift" it to being > public. Though regarding comments I've made earlier about having annotation data statically > available at runtime, if that were added to dart, you could export _pi like this.

Alternatively, we could allow part template.g.dart in which case private classes, enums, methods, etc would all be available to the template.

Also:

any reason not to allow all things dart not currently allowed in templates?

  • await
  • constructors
  • anonymous functions
  • cascades
  • generics
  • interpolation
  • assignment
  • '??'
  • this/super
  • throw/retthrow
  • named arguments
  • is checks
  • as casts

?

You tell me :) We'd like to transition the compiler to use angular_ast, but that is yet another migration (TM). I'd like to prioritize this, though, and stop using the current expression parser. Maybe I can work with you or Max to do this earlier.

@MichaelRFairhurst
Copy link
Contributor

Imports may be used only for the template and would show up as analyzer hints.

Ah, yes, that's true. I'd be part tempted to include imports in the template, myself, to solve this problem. But I tend to really give angular a lot of credit in being its own unique language rather than a framework...the frameworkier side of thought aligns better with an exports field.

Also could create problems with testing, if your template itself had the ability to import production code. Though if most of what its importing is enums, that certainly would be unlikely to create problems.

Other questions for the export field, any plans to do aliases?

  exports: const [ const Export(Foo, as: "bar") ]

note: as: won't work here, but for the sake of assuming we get some clean name, I will keep using as:.

I will say, I get a bit of a funny feeling because I think this general concept is so far conflating values with identifiers. Sure, if I don't like the default name, or have a calculation I want to give a name (like min(foo, bar) I could do so with something akin to const Export(..., as: ...).

But what if I do

const x = const Export(y, as: 'b')
...
  exports: [x]

Now is it accessible in my template as x, b, or y?

And what if I do

  exports: [ALL_MY_ENUMS]

Is it an exported identifier or an exported array of exports? Can I do OneOfMyEnums.foo in my template? Or do I need to do ALL_MY_ENUMS.OneOfMyEnums.foo? Or ALL_MY_ENUMS[x].foo?

And if its the formost, what happens when I export an array of Exports that includes an export with no name? Or a single value in the array isn't an Export?

Some fuzzy stuff here, could get confusing to spec out, and could get confusing to use. I guess my thought is that dart imports have already solved this problem in a clear way.

@matanlurey
Copy link
Contributor Author

@MichaelRFairhurst:

Imports may be used only for the template and would show up as analyzer hints.

Ah, yes, that's true. I'd be part tempted to include imports in the template, myself, to solve this problem. But I tend to really give angular a lot of credit in being its own unique language rather than a framework...the frameworkier side of thought aligns better with an exports field.

We did consider this, i.e.

<import src="messages.dart" />

That did seem like a (1) lot more work and (2) introduced more non-standard HTML to understand.

Also could create problems with testing, if your template itself had the ability to import production code. Though if most of what its importing is enums, that certainly would be unlikely to create problems.

Other questions for the export field, any plans to do aliases?

  exports: const [ const Export(Foo, as: "bar") ]

We should be able to support import aliases, for example:

import 'messages.dart' as messages;

exports: const [
  messages.FooMessages
],
<div [title]="messages.FooMessages.someTitle"></div>

I think that would work for most cases, no?

note: as: won't work here, but for the sake of assuming we get some clean name, I will keep using as:.

I will say, I get a bit of a funny feeling because I think this general concept is so far conflating values with identifiers. Sure, if I don't like the default name, or have a calculation I want to give a name (like min(foo, bar) I could do so with something akin to const Export(..., as: ...).

But what if I do

const x = const Export(y, as: 'b')
...
  exports: [x]

Now is it accessible in my template as x, b, or y?

And what if I do

  exports: [ALL_MY_ENUMS]

Is it an exported identifier or an exported array of exports? Can I do OneOfMyEnums.foo in my template? Or do I need to do ALL_MY_ENUMS.OneOfMyEnums.foo? Or ALL_MY_ENUMS[x].foo?

And if its the formost, what happens when I export an array of Exports that includes an export with no name? Or a single value in the array isn't an Export?

Some fuzzy stuff here, could get confusing to spec out, and could get confusing to use. I guess my thought is that dart imports have already solved this problem in a clear way.

@alan-knight
Copy link

alan-knight commented May 12, 2017 via email

@MichaelRFairhurst
Copy link
Contributor

MichaelRFairhurst commented May 12, 2017

It still feels weird to me, because import x and class y etc, are all syntactically exceptional constructs that deal with identifiers and constructs.

It seems weird to me that an array could do similar things, because arrays are values and not identifiers. Its also not composable, meta-programmable, because you can't merge two arrays of identifiers. You get into the question of intent, was this intended to be read as a value or not?

But enough of me saying "hey I gave a weird feeling". Here's what I agree with:

  • extending html even more can create its own problems, for sure
  • the amount of aliasing which will occur will likely be very small
  • supporting imports which are aliased in the dart file to also being aliased in the html file would be good

For instance, my ALL_MY_ENUMS could be a dart file that exports each of my_enum.dart files. If you want to do ALL_MY_ENUMS.SomeEnum.foo in your template, import all_my_enums.dart as ALL_MY_ENUMS. If you want to do SomeEnum.foo, then simply drop the alias.

What about a radical idea, annotating the imports?

@Expose
import 'enums.dart`;

@Component(selector: 'foo', template: '{{MyEnum.foo}}')
class UsesEnumComponent {
  ...
}

this does still result in unused import warnings, but I would consider that a bug.

I do dislike the level of disconnect between the component and what's exposed to it. But it feels less boiler-plate-y to me, and it no longer conflates values with identifiers.

Edit:

I was mistaken about easily supporting all enums via all_my_enums.dart.

If you annotate the import, its easy:

@Expose
import 'all_my_enums.dart';

@Component(selector: 'foo', template: '''
    {{MyFirstEnum.foo}} {{MySecondEnum.foo}} {{MyThirdEnum.foo}}
''')
class ....

and I would consider this extendible/metaprogrammable/composable, because all it takes is adding MyFourthEnum as an export of all_my_enums to get it everywhere in your project.

However, if an exports array is required,

import 'all_my_enums.dart';

@Component(
  selector: 'foo',
  exports: [MyFirstEnum, MySecondEnum, MyThirdEnum],
  template: '''
    {{MyFirstEnum.foo}} {{MySecondEnum.foo}} {{MyThirdEnum.foo}}
''')
class ....

then there's no way to avoid repeating yourself without getting back into arrays of Export instances which then become questionable because they too are values referenced by identifiers most of the time.

And when MyFourthEnum is added to all_my_enums.dart, you don't get it for free in every template. You still have to add it to the exports array.

@MichaelRFairhurst
Copy link
Contributor

MichaelRFairhurst commented May 12, 2017

If we go with imports inside the templates, I'd probably recommend

{{import 'foo.dart'}}

over

<import src="foo.dart">

in terms of purely opinionated points of discussion :)

@zoechi
Copy link

zoechi commented May 15, 2017

I would prefer keeping imports out of HTML

@matanlurey
Copy link
Contributor Author

Giving a small update: We have a proof of concept of this working, mostly, so it's likely we'll go with the exports: const [ ... ] for any initial implementation. Thanks folks for weighing in!

@harryterkelsen harryterkelsen self-assigned this May 22, 2017
@matanlurey
Copy link
Contributor Author

Fixed by 0a28527 and 1b44398.

Woohoo!

@stevenroose
Copy link

Does this also solve the case for static component class members?

@matanlurey
Copy link
Contributor Author

Yup: #911 (comment)

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

No branches or pull requests

10 participants