dart-archive / web-components

Dart package providing the web components platform polyfills
https://pub.dartlang.org/packages/web_components
BSD 3-Clause "New" or "Revised" License
18 stars 10 forks source link

Allow @HtmlImport() without library directive #40

Open munificent opened 8 years ago

munificent commented 8 years ago

Right now, that annotation will only be found on a library directive. Given that those directives are otherwise not very useful, it would be nice if users could omit them.

This probably means looking for @HtmlImport before any directive and, if there are none, on the first declaration in the file. I believe the test package does something similar for @TestOn, so @nex3 is a good person to ask about this.

nex3 commented 8 years ago

In test, we look for top-level annotations on the first directive in the file—the library tag if it exists, and the first import otherwise. We just use the analyzer's parseDirectives() function to get at them.

jakemac53 commented 8 years ago

I definitely agree this would be nice

jakemac53 commented 8 years ago

cc @sigmundch

So actually this is a bit more complicated that it would first appear. This is an Initializer (from package:initialize) which means its initialize method gets called with something specific based on what was annotated.

Currently only libraries, top level methods, and classes are supported. We could certainly add support for annotating imports, but @HtmlImport today relies on getting a LibraryDescriptor object in its initialize method, which it uses for calculating relative import paths. If an import was annotated it would probably get an ImportDescriptor which would most likely not contain any info about the surrounding library.

I think probably the most reasonable solution here would be two fold:

  1. Make an additional type of Initializer (maybe a LibraryInitializer?) which always gets called with an additional argument, a LibraryDescriptor corresponding to the surrounding library where the annotation appears.
  2. Allow Initializers on all top level declarations. This means adding support for at least import, export, part, part of, and top level variables. Many of these would be useful anyways so it probably wouldn't hurt to have.

This would also have the benefit of allowing people to move their @HtmlImport annotations to their polymer element classes.

sigmundch commented 8 years ago

Yeah, this unfortunately requires some extra changes in initialize like @jakemac53 suggests.

If an import was annotated it would probably get an ImportDescriptor which would most likely not contain any info about the surrounding library.

I guess it could be possible to add to the ImportDescriptor some details about the enclosing library or even a pointer to a LibraryDescriptor, right?

jakemac53 commented 8 years ago

I guess it could be possible to add to the ImportDescriptor some details about the enclosing library or even a pointer to a LibraryDescriptor, right?

Yes but that would just enable it to work for imports. I don't like the idea of adding a LibraryDescriptor just for that case and not others, hence the idea of a different Initializer type which always gets a LibraryDescriptor in addition to the regular things.