Request to refactor to add option to not overwrite the entry-point file about reflectable.dart

[sigurdm]

I think it is hard to avoid relying on barback at all.
We are relying quite heavily on https://github.com/dart-lang/code_transformers that again relies on barback.
We could make an interface that can be accessed without pub - but the barback dependency itself will be hard to get rid of without rewriting a lot of stuff.

[sethladd]

cc @kevmoo

Thanks for the update, @sigurdm

[jakemac53]

Is Flutter willing to accept any build step which does source generation? I thought from previous conversations that they were not.

[sethladd]

Ideally, no build step. But... happy to explore near-term options while we work on a long-term option.

Near-term option can be a "generate code to disk" thing, assuming that's a utility we can wire into the Flutter tools that are emerging.

e.g. some watcher that watches a dir and kicks off a source gen might be perfectly sufficient.

[jakemac53]

OK, good to know. This probably has to wait for whatever generalized source-gen solution we come up with.

[eernstg]

I guess the rule would be "for practical reasons we don't want to run two
translators". Otherwise I don't really see how running a pipeline of one
reflectable transformer (possibly a stand-alone variant, if we decide that
it's a good idea to build such a thing) followed by one compiler would be
different-in-principle from running one compiler that happens to perform
the reflectable transformation as one of the compiler phases. In that case
it's a matter of tool maturity rather than a matter of "we have the
principle of rejecting all transformers". ;-)

On Tue, Sep 22, 2015 at 6:37 PM, Jacob MacDonald [email protected]
wrote:

OK, good to know. This probably has to wait for whatever generalized
source-gen solution we come up with.

—
Reply to this email directly or view it on GitHub
#35 (comment)
.

Erik Ernst - Google Danmark ApS
Skt Petri Passage 5, 2 sal, 1165 København K, Denmark
CVR no. 28866984

[sethladd]

I don't know how to use reflectable today without pub serve (requires I opt-into pub's web server, which we can't do because we have our own web server) or pub build (which is a "heavy-weight" batch build processor).

[jakemac53]

You can use barback outside of pub serve, its just cumbersome (you have to manually supply it with all files). The code-transformers package does this to run transformer tests.

If you were willing to take a Barback dependency, you could build a separate tool which uses the existing reflectable transformer but writes to disk.

[zoechi]

The transformers modify files inline AFAIK (without giving them another name). This would probably be cumbersome when the transformed code should be persisted to disk. You probably don't want to overwrite the user-created source files.

[jakemac53]

I think that the reflectable transformer only rewrites a single file - the entry point. This could easily be modified to bootstrap instead like the polymer transformers do.

[zoechi]

I see. Then it should work out.

[sethladd]

This could easily be modified to bootstrap instead like the polymer transformers do.

oooh, nice! If reflectable can be modified in this way, perhaps that's what we need to hack together a prototype reflectable without pub serve.

[jakemac53]

It should be possible to provide this as an option? In some ways not bootstrapping is nice (plays nice with other transformers, don't have to rewrite the script tag in the html).

[sethladd]

@jakemac53 good point. Clarified title.

[sigurdm]

Let me be sure I understand.
If we have entry point A.dart you want the result of the translation to be
A.bootstrap.dart (or similar) that can be used as an alternative entry point?

This is certainly possible.

There is one more complication though.
Currently we have a second transformer (src/transform_import.dart) that switches the import of
reflectable_mirror_based.dart for an import of reflectable_transformer_based.dart in reflectable.dart.
This is necessary to avoid the dart:mirrors dependency after transformation. This step would also have to be done as part of the stand-alone transformation to make everything work.

[eernstg]

Reflectable is not overwriting any files, because each file which is
transformed is copied or transformed from the source directory tree (e.g.,
'test/my_test.dart') to the target directory tree
('build/test/my_test.dart'). A transformer can let a file pass through from
the former to the latter without any changes, and it can receive the file
(asset) and create a transformed version, and it can also do other things
such as creating additional assets with entirely new names.

So we're talking about the relation between the files in the source tree
and the target tree rather than the relation between a set of files before
and after a certain point in time (which would be the natural
interpretation of "the reflectable transformer overwrites a file").

But if you consider the source tree and the target tree to be the same
files in some sense then you could say that reflectable is "overwriting"
the entry point file: If we transform an entry point, say
'web/index.dart', then we'll output a file
'build/web/index_reflectable_original_main.dart' with the same contents,
and then we'll generate a file (no client code, it's all generated stuff)
'build/web/index.dart'. Other files are transferred without modification
(so, in particular, if they import 'index.dart' then they'll get the
generated library, but we have inserted imports/exports into the generated
library such that the name space is as expected by the client).

The point is that we need to intercept the main function, such that
invocation of 'build/web/index.dart' will actually do some initialization
first, and then call the original main function. It is not a problem if
the new main is called multiple times, because the initialization is
idempotent and extremely cheap after the first time (and it proceeds to
call the original main, so clients will get the original behavior).

With this renaming/generation scheme you can transform an entry point like
'web/index.dart' and its dependencies and then use 'build/web/index.dart'
as your transformed entry point. If you directly use the original entry
point ('build/web/index_reflectable_original_main.dart') then you will see
a runtime error as soon as you use anything reflectable, because the
initialization was bypassed.

Why is it important for you to avoid letting the generated library have the
name of the original entry point? As far as I can see it only forces
clients to know that they must use a different name after transformation.
The transformed program will not run (if it actually uses reflectable, which
would not be an unreasonable assumption here) if the original entry point
is used as the entry point (that is, you cannot "run the program in untransformed
mode" or anything similar just by using that library as the entry point). In other
words, if 'build/web/index.dart' must be identical to 'web/index.dart' and the
generated library should be named 'build/web/index.bootstrap.dart' then
clients would have to remember that 'web/index.dart' must be avoided and
'build/web/index.bootstrap.dart' must be used as the entry point. How would
that help anyone?

But it would be very easy, of course, to create an additional file whose name
includes '.bootstrap.' such that clients who want to use that file name could do
it: that would simply be a symlink to or a copy of the generated entry point.

On Tue, Sep 22, 2015 at 10:33 PM, Seth Ladd [email protected]
wrote:

This could easily be modified to bootstrap instead like the polymer
transformers do.

oooh, nice! If reflectable can be modified in this way, perhaps that's
what we need to hack together a prototype reflectable without pub serve.

—
Reply to this email directly or view it on GitHub
#35 (comment)
.

Erik Ernst - Google Danmark ApS
Skt Petri Passage 5, 2 sal, 1165 København K, Denmark
CVR no. 28866984

[sethladd]

If we have entry point A.dart you want the result of the translation to be A.bootstrap.dart (or similar) that can be used as an alternative entry point?

Yes, please! That would be excellent.

[jakemac53]

There is one more complication though.
Currently we have a second transformer (src/transform_import.dart) that switches the import of
reflectable_mirror_based.dart for an import of reflectable_transformer_based.dart in reflectable.dart.

Good point.... we are going to need to figure out a way to solve this issue elsewhere as well.

[eernstg]

The stand-alone transformer that we included in 0.5.2 may be relevant for this issue. It uses barback, but runs outside pub. It will produce the transformed file for a specified entry point, but it will not copy over all the files that remain unchanged (it does the transformation work only, not the default actions taken by pub when no transformation is needed), so it is not a plug-and-play solution to the request made in this issue—but it might still be a useful step in that direction.

[jakemac53]

We now have the build package, so it would be great if we could integrate with that. @eernstg we should talk offline about what it would take to do this.

The main issue I see right now is the rewriting of the reflectable_mirror_based.dart import to reflectable_transformer_based.dart. It is unclear to me exactly how to get around that if reflectable still wants to support a mirrors based version, so we should do some brainstorming.

[eernstg]

I guess we could simply have two variants of 'reflectable.dart' (sharing most of the text using 'part of'), and then one would be used the way it is today ('reflectable.dart') and the other could be used when no pre-transform execution is required and no transformation should take place ('static_reflectable.dart'). A specialized reflectable transformer would then just generate the new main file (producing 'main.bootstrap.dart' based on the entry point 'main.dart'), and no files would be copied. Since every import of reflectable in the transitive closure of 'main.dart' would use 'static_reflectable.dart' there is no need to change import 'reflectable_mirror_based.dart' to import 'reflectable_transformer_based.dart', because we already have the latter. In essence, this new variant of the reflectable transformer would do nothing else than generating one new file. But the program would not be runnable at all before transformation.

[jakemac53]

The problem with that is then if any package you depend on imports reflectable.dart it doesn't work. We could break out the bootstrapping logic into a separate import though (this is what angular does). So we could have reflectable_bootstrap.dart and reflectable_bootstrap_static.dart. Only the non-static bootstrap import would contain an import to dart:mirrors, and then you would be able to opt in/out of mirrors at the application level.

Of course this is a breaking change, but its a very easy one to deal with (simply changing one import in your entry points).

[eernstg]

I guess "you" is the client package 'client' who puts 'reflectable' into the dependencies in pubspec.yaml? So when 'client' also depends on 'awesome_thing' which imports 'reflectable.dart' then we're in trouble, because we sometimes want 'awesome_thing' to import 'static_reflectable.dart' instead. That's true.

But I don't think the fix you mention will work so easily: The actual behavior of reflectors comes about because we have abstract class Reflectable extends implementation.ReflectableImpl .., i.e., reflectors get their actual behavior (which is based on 'dart:mirrors' or on transformer-generated mirror like objects, depending on which import is in effect), and redirecting an extends to a different class is not something a bootstrap operation could do (assuming that bootstrapping is something that a Dart program can do at runtime).

Or maybe that's just because I haven't quite understood where you're going. ;-)

[jakemac53]

Ah ya I didn't realize that is how reflectable is working. Will have to come up with something else then.

[eernstg]

Closing: The requested property is satisfied with reflectable of today (it uses the build package, and code generation using build never overwrites existing files).