Angular: Rename bootstrap -> startApp (and related work)

[matanlurey]

As @nshahan found out migrating angular_components_example:

Uncaught Error: Unsupported operation: Using the 'angular' transformer is required.

Please see https://webdev.dartlang.org/angular/tutorial for setup instructions,
and ensure your 'pubspec.yaml' file is configured to invoke the 'angular'
transformer on your application's entry point.

As of 5.0.0-alpha+2, we no longer rewrite bootstrap, so we should fix the error message to say that using bootstrapStatic directly is now required. Optionally we could also deprecate bootstrap _or_ we could change bootstrap and have that _replace_ bootstrapStatic.

(For a transitional period bootstrap would still have to throw if missing initReflector, though)

Thoughts? @alorenzen @leonsenft @nshahan

[leonsenft]

Since bootstrap is no longer supported, how about we reclaim it and use it for what is currently bootstrapFactory?

[nshahan]

Would you be able to recognize when users haven't updated their code and give them a helpful message instructing them to do so?

[matanlurey]

@leonsenft:

Since bootstrap is no longer supported, how about we reclaim it and use it for what is currentlybootstrapFactory?

Seems confusing to do in one release, but open for discussion.

The issue also is bootstrapStatic also sounds like it is, well, static, even though it isn't.

---

@nshahan:

Would you be able to recognize when users haven't updated their code and give them a helpful message instructing them to do so?

Depends what you mean. We _could_ scan entry_points (https://github.com/dart-lang/angular/issues/757), and check to see that they are using bootstrapStatic directly, correctly. An easier option would be just improving the error message, though:

ComponentRef<dynamic> bootstrap(Type type, List providers, [Function initReflector]) {
  if (initReflector == null) {
    throw 'initReflector is required as of AngularDart 5.0.0-alpha+2... <YADA YADA>';
  }
}

[leonsenft]

Seems confusing to do in one release, but open for discussion.

Even if we removed bootstrap in one version, then brought it back for fast boot in the next, you still might have customers skipping the version in which it was removed, and experience the confusing API change during an upgrade.

Is getting a type error on the parameters to an existing function, or a function being removed a more disruptive change? At least with the parameter error, they can click through to the updated documentation to see a useful comment explaining how to use the new API.

The issue also is bootstrapStatic also sounds like it is, well, static, even though it isn't.

Would you prefer aliasing bootstrapStatic as bootstrap then?

[matanlurey]

Anybody have more opinions here?

@leonsenft @alorenzen @hterkelsen @kevmoo

[kevmoo]

No strong preference...

[matanlurey]

I think we should probably just toss the word "bootstrap". Perhaps:

startAngularApp<T>(ComponentFactory<T>)?

and renaming bootstrapStatic to startAngularAppLegacy? It's not quite deprecated.

[hterkelsen]

We shouldn't have the redundant "Angular" in the name. How about startApp? Whatever we do, I think we should have the "good" (the entrypoint we would prefer our users use) be just called startApp and the other one called startAppLegacy rather than being in a situation where we have startApp for the old way, and startAppStatic for the new improved way.

[kevmoo]

+1 to what Harry said...

On Wed, Jan 24, 2018 at 4:16 PM, Harry Terkelsen [e-mail]
wrote:

We shouldn't have the redundant "Angular" in the name. How about startApp?
Whatever we do, I think we should have the "good" (the entrypoint we would
prefer our users use) be just called startApp and the other one called
startAppLegacy rather than being in a situation where we have startApp
for the old way, and startAppStatic for the new improved way.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dart-lang/angular/issues/756#issuecomment-360318424,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABCiqlr9fTraXucbr3ayMQebzz_EczRks5tN8fsgaJpZM4RHr5F
.

[matanlurey]

I like startApp and startAppLegacy. If I don't get other feedback, lets go for that!

[alorenzen]

+1 for startApp.

I'm only in support of startAppLegacy if we are actively considering it deprecated and pending removal once everyone migrates off of it. If the plan is to keep startAppLegacy for >1 year, I think we should consider something more descriptive than legacy.

[matanlurey]

Strictly speaking, the _Legacy_ components are the following:

• Almost any practical use of ReflectiveInjector.
• Every use of SlowComponentLoader and it's "friend" ComponentResolver.

I think it's very feasible to get rid of SlowComponentLoader and ComponentResolver in <1 year, maybe even one more quarter, there are ~100 instances of use remaining internally, and some of these exist entirely as a migration aid for teams rapidly moving off of it (@TedSander FYI).

ReflectiveInjector is a bit more tricky, it has ~200 instances of use, and the replacement API, @Injector.generate isn't stable yet, and might not encompass 100% of use cases. Additionally, it is used for _every_ test (both internally and externally), though not explicitly. The testing case is not as important though, because you don't use startApp*, and we could support ReflectiveInjector "for testing" more long term.

Fine with a more descriptive name:

• startAppWithReflectiveSupport
• startAppReflectiveInjectorSupported
• startAppWithCodeSizePenalty

[alorenzen]

Are both ReflectiveInjector and SlowComponentLoader marked as @Deprecated (or could be in the near future)? If so, then I'm fine with startAppLegacy.

[matanlurey]

SlowComponentLoader is ready to be deprecated (and ComponentResolver is already):
https://github.com/dart-lang/angular/blob/54cac9c2c0a588cf1b0f5a00fde51be1e00eb81e/angular/lib/src/core/linker/dynamic_component_loader.dart#L12-L14 ... I'll send a CL out today making this change.

I don't think I can in good faith deprecate ReflectiveInjector yet. I'm working with a few folks internally on remaining edge cases, and that's not even including the work needed for testing or elsewhere.

[TedSander]

@matanlurey just a note that those ~100 instances do fan out a pretty good amount since they are used in some big primitives. I think under a year is a very reasonable timeline. A quarter/under a quarter might need a pretty big push by framework members to finish it off.

[matanlurey]

@TedSander Fair enough. Still reasonable to call it "legacy", or should we wait?

[chalin]

On a related note: will manually importing of the template (e.g. import 'main.template.dart' as ng) still be necessary or is that going away? It is currently used to call initReflector().

[matanlurey]

The startApp<With|Legacy|Deprecated> variant will require it.

The "recommended default", will not.

[leonsenft]

@matanlurey Won't it still be required to import the generated file to use the generated component factory?

[matanlurey]

Err, yes, just not for initReflector.

[chalin]

Any progress on this?

[matanlurey]

No, not yet.

[matanlurey]

This is complete as of HEAD. We'll have CHANGELOG entries/docs on this soon.